GCC warning: variable 'temp' set but not used

[J. V. (janvi)]

obenstehende Warnung kommt nach Umstieg auf eine aktuelle Arm GCC 
Version. Der Quellcode ist

void ResetRx1 (void)
{
 volatile int8_t temp;
 temp = USART1->SR;                // SR-DR Lesesequenz setzt alle Bits des Empfaengers zurueck
 temp = USART1->DR;                // das sind RXNE, IDLE, ORE Overrun Error, NE Noise Error, FE Frame Error, PE Parity Error
 Rx1WrPtr = 0;                  // Erklaere Empfangspuffer fuer ungueltig
 Rx1RdPtr = 0;                  // Lesezeiger auf Anfang stellen
USART_ITConfig(USART1, USART_IT_RXNE, ENABLE);  // Maske fuer Empfangsinterrupt aufschalten
}

Eigentlich habe ich temp extra auf volatile gesetzt weil es nicht mehr 
zurückgelesen wird. Warum dann die Warnung ?

[leo (Gast)]

J. V. schrieb:
> Eigentlich habe ich temp extra auf volatile gesetzt

... aber lokal delariert, i.e. du kannst nur innerhalb der Sub auf sie 
zugreifen.

leo

[Thomas F. (tommf)]

Das volatile bewirkt hier nichts, da die Variable auf dem Stack liegt 
und damit ihre Adresse ausserhalb der Funktion nicht bekannt und auch 
nicht ermittelbar ist. Der Compiler ist also clever und glaubt dir 
nicht.

In GCC kannst du aber die Variable direkt als unused deklarieren:

int8_t temp __attribute__((unused));

Das ist dann aber nicht auf andere Compiler übertragbar.

[Schlaumaier (Gast)]

temp = USART1->SR;


Dir fehlt z.b. ein  xxx = temp oder so was ähnliches

Einfach gesagt. Und weißt einer Variable einen wert zu ABER du benutzt 
diese Variable im Code (dieser Sub) nicht mehr.

Ist du Variable Public deklariert, muss du sie irgendwo im Code wieder 
Auslesen o. benutzen.
Ist sie es nicht, musst du sie in der Sub (hier void) benutzen.

Das ist eine "Hast du nicht was vergessen" Warnung.

[Jonas B. (jibi)]

>... aber lokal delariert, i.e. du kannst nur innerhalb der Sub auf sie
>zugreifen.

>leo

Er benutzt temp schlicht nicht. Den Rest interpretierst du wieder mal 
rein.

[Jörg W. (dl8dtl) (Moderator)]

Thomas F. schrieb:
> In GCC kannst du aber die Variable direkt als unused deklarieren:

Dazu muss man nicht erst in die GCC-Trickkiste greifen. Geht einfach so:

void ResetRx1 (void)
{
 (void)USART1->SR;                // SR-DR Lesesequenz setzt alle Bits des Empfaengers zurueck
 (void)USART1->DR;                // das sind RXNE, IDLE, ORE Overrun Error, NE Noise Error, FE Frame Error, PE Parity Error
 Rx1WrPtr = 0;                  // Erklaere Empfangspuffer fuer ungueltig
 Rx1RdPtr = 0;                  // Lesezeiger auf Anfang stellen
USART_ITConfig(USART1, USART_IT_RXNE, ENABLE);  // Maske fuer Empfangsinterrupt aufschalten
}

[leo (Gast)]

Jonas B. schrieb:
> Er benutzt temp schlicht nicht.

... nur schreibend, ja. Ist ja offensichtlich.

> Den Rest interpretierst du wieder mal
> rein.

What? Welchen Rest? Was sollte ich da interpretieren?

leo

[Jonas B. (jibi)]

>Was sollte ich da interpretieren?

Das er außerhalb drauf zugreift.

>i.e. du kannst nur innerhalb der Sub auf sie
>zugreifen.

[Bauform B. (bauformb)]

Jörg W. schrieb:
sinngemäß
> Dazu muss man nicht erst in die GCC-Trickkiste greifen.
> (void)USART1->SR;  // SR-DR Lesesequenz setzt alle Flags zurueck
> (void)USART1->DR;  //

wie offiziell ist denn das?
Mein gcc 9.2 erzeugt übrigens auch ohne (void) den richtigen Code. Wie 
offiziell wäre denn das?

[Jörg W. (dl8dtl) (Moderator)]

Bauform B. schrieb:

> wie offiziell ist denn das?

Ganz und gar :-)

> Mein gcc 9.2 erzeugt übrigens auch ohne (void) den richtigen Code. Wie
> offiziell wäre denn das?

Genauso. Kann aber halt sein, wenn man die Warnungen besonders pingelig 
stellt, dass einem dann jemand sagt, dass man das Ergebnis des Ausdrucks 
nicht benutzt. Mit dem Cast nach "void" sagt man ganz explizit "ich 
weiß, dass es hier einen Wert gibt, aber ich möchte den wegwerfen". Ist 
natürlich auch zur Dokumentation sinnvoll, das erübrigt dann Kommentare 
der Form "Register nur lesen, aber Wert wegwerfen". Das "(void)" drückt 
das Ganze viel kürzer aus. ;-)

[J. V. (janvi)]

> (void)USART1->DR;

Als GCC Optimierer hätte ich sowas dann ganz wegoptimiert womit es dann 
aber auch nicht mehr funktioniert wenn der Optimizer Schalter was 
anderes als -O0 ist. Aber mit -Wall bin ich wohl auch selbst etwas 
schuld am rumgemeckere. Morgen schau ich mir mal die globalen Variablen 
an ob ich davon nicht was missbrauchen kann.

[Jörg W. (dl8dtl) (Moderator)]

J. V. schrieb:
> Als GCC Optimierer hätte ich sowas dann ganz wegoptimiert

Wenn der Trottel, der die UART-Register definiert hat, sie nicht 
"volatile" deklariert hat, dann hast du Recht.

Aber da derjenige wahrscheinlich kein Trottel war, steht es dem 
Optimierer halt nicht frei, diesen Teil wegzuwerfen, sondern er muss ihn 
schön brav ausführen und nur das Ergebnis verwerfen.

> Aber mit -Wall bin ich wohl auch selbst etwas schuld am rumgemeckere.

Nein.

Industriestandard bei uns in der Firma: -Wall -Wextra -Werror.

Wenn noch weitere sinnvolle Warnungen entdeckt werden, kommen diese gern 
noch dazu.

(Wenn tatsächlich irgendwas nicht mehr frei von Warnungen damit 
compiliert, kann man sich als Ausnahme immer noch lokal mit einem Pragma 
helfen. Da steht dann aber auch 'ne Begründung dabei, warum das jetzt 
gerade so ist.)

> Morgen schau ich mir mal die globalen Variablen an ob ich davon nicht
> was missbrauchen kann.

Mach doch nicht so'n Käse. Schreib den Typecast nach "void" davor, wie 
das jeder andere Embedded-Programmierer auch tut, und fertig ist die 
Laube – und das auch noch portabel (so portabel, wie Controller-Code 
halt sein kann).

[J. V. (janvi)]

Ok, klar. Das volatile gehört natürlich an die Registerdefinition ran. 
Die hat im Übrigen STM gemacht und das schau ich mir morgen genauer an 
ob das so ok geht. Und -Wall bleibt auch wie immer dran

[Yalu X. (yalu) (Moderator)]

J. V. schrieb:
> Eigentlich habe ich temp extra auf volatile gesetzt weil es nicht mehr
> zurückgelesen wird.

Das volatile ändert nichts an der Tatsache, dass du den Inhalt von temp
nicht benutzt, zwingt den Compiler aber, die Variable tatsächlich
anzulegen, und zwar nicht etwa als Register, sondern im Hauptspeicher
auf dem Stack. Dadurch wird nicht nur Stack-Speicherplatz verschwendet,
der Compiler erzeugt darüberhinaus zwei unnötige Schreibzugriffe auf den
Speicher und je nach Plattform zusätzlich noch Code für die Reservierung
der Variable auf dem Stack.

Ohne das volatile würde der Compiler die Variable und die Zugriffe
darauf einfach wegoptimieren, aber die Warnung bleibt natürlich
(zurecht) bestehen.

Lass das temp deswegen einfach komplett weg und folge Jörgs Rat mit dem
void-Cast.

[Jörg W. (dl8dtl) (Moderator)]

J. V. schrieb:
> Die hat im Übrigen STM gemacht und das schau ich mir morgen genauer an
> ob das so ok geht.

Sicher geht das so OK. Ohne "volatile" würden all diese 
Hardware-Register schlicht nicht so funktionieren wie gewünscht, weil 
einem der Optimierer laufend in die Suppe spuckt – und genau das war ja 
auch der Grund, sowas wie "volatile" überhaupt einzuführen. Schon bei 
der PDP-11 (von der C her kommt) war es gang und gäbe, dass 
Geräteregister memory-mapped waren, also letztlich über Zeiger 
zugegriffen werden mussten.

Bei STM sieht das ungefähr so aus. MCU-spezifische Datei:

#define USART1              ((USART_TypeDef *) USART1_BASE)
// …
typedef struct
{
  __IO uint16_t SR;         /*!< USART Status register,                   Address offset: 0x00 */
  uint16_t      RESERVED0;  /*!< Reserved, 0x02                                                */
  __IO uint16_t DR;         /*!< USART Data register,                     Address offset: 0x04 */
  uint16_t      RESERVED1;  /*!< Reserved, 0x06                                                */
  __IO uint16_t BRR;        /*!< USART Baud rate register,                Address offset: 0x08 */
  uint16_t      RESERVED2;  /*!< Reserved, 0x0A                                                */
  __IO uint16_t CR1;        /*!< USART Control register 1,                Address offset: 0x0C */
  uint16_t      RESERVED3;  /*!< Reserved, 0x0E                                                */
  __IO uint16_t CR2;        /*!< USART Control register 2,                Address offset: 0x10 */
  uint16_t      RESERVED4;  /*!< Reserved, 0x12                                                */
  __IO uint16_t CR3;        /*!< USART Control register 3,                Address offset: 0x14 */
  uint16_t      RESERVED5;  /*!< Reserved, 0x16                                                */
  __IO uint16_t GTPR;       /*!< USART Guard time and prescaler register, Address offset: 0x18 */
  uint16_t      RESERVED6;  /*!< Reserved, 0x1A                                                */
} USART_TypeDef;

Referenziert wird hier __IO, und das kommt aus den ARM-eigenen 
Definitionen. Die sehen so aus:

#ifdef __cplusplus
  #define   __I     volatile             /*!< Defines 'read only' permissions                 */
#else
  #define   __I     volatile const       /*!< Defines 'read only' permissions                 */
#endif
#define     __O     volatile             /*!< Defines 'write only' permissions                */
#define     __IO    volatile             /*!< Defines 'read / write' permissions              */

Da hast du nun dein "volatile". ;-)

[J. V. (janvi)]

Aha, Danke - da muss ich nicht mal mehr selbst suchen.

[A. S. (Gast)]

J. V. schrieb:
> Aha, Danke - da muss ich nicht mal mehr selbst suchen.

Müssen nicht. Tu es trotzdem. Einfach weil es wichtig ist. Normalerweise 
sollte dein Editor das anzeigen, wenn Du mit der Maus drüber bist. Falls 
nicht, bzw. falls es mehr als 4 Sekunden dauert, das zu finden, dann 
denke über eine IDE nach.

[J. V. (janvi)]

>falls es mehr als 4 Sekunden dauert, das zu finden,
>dann denke über eine IDE nach

Ja das habe ich auch schon. Der Wunschkandidat heisst Eclipse.
Das CDT Plugin scheint es ja auch in einer Embedded Version zu
geben, die dann auch noch das Segger Kabel bzw. Ozone unterstüzt.
Bis dahin gibt es aber noch ein paar weitere Nüsse zu knacken.

[Blume (Gast)]

J. V. schrieb:
> Ja das habe ich auch schon. Der Wunschkandidat heisst Eclipse.
> Das CDT Plugin scheint es ja auch in einer Embedded Version zu
> geben, die dann auch noch das Segger Kabel bzw. Ozone unterstüzt.
> Bis dahin gibt es aber noch ein paar weitere Nüsse zu knacken.

oder CLion ???