Compiler warnings (Part 1)

Part 1

By default, when compiling a project with the Arduino IDE, the compiler will only complain if an error occurs  and quietly ignore warnings. Unlike an error, a warning will let you built an application but it may induce a bug. However, an application which contains a warning may be fully functional and  an application with no warning may be buggy! In some cases, (e.g. debugging an application), it may be useful to show warnings too.  The Arduino software will show all of the compiler output, including the warnings if you check “compilation” in File>Preferences>”show verbose output during:” as per the following illustration:

verbose

Now, the Arduino IDE shows the warnings from your projects but also from the Arduino’s built-in libraries among which the very popular HardwareSerial library. Did you ever used it? I’m almost sure that you used it a lot. Remember the  Serial object (e.g. Serial.begin(), Serial.print()…): it is part of the HardwareSerial library. This library is fully operational but using the verbose option while compiling shows  multiple warnings; and it is a little bit annoying to see the following warnings in red in a long list of warning free commands:

C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp: In function 'void store_char(unsigned char, ring_buffer*)':
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp:98: warning: comparison between signed and unsigned integer expressions
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp: In function 'void __vector_18()':
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp:127: warning: unused variable 'c'
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp: In member function 'void HardwareSerial::begin(long unsigned int, byte)':
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp:368: warning: unused variable 'current_config'
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp: In member function 'virtual size_t HardwareSerial::write(uint8_t)':
C:\Program Files (x86)\Arduino\hardware\arduino\cores\arduino\HardwareSerial.cpp:467: warning: comparison between signed and unsigned integer expressions

There are 2 groups of 2 warnings:

First group: 2 of them (line 98 and 467) are about a “comparison between signed and unsigned integer expressions”. The following example shows a case where it would be pertinent:

uint8_t twoHundred = 200;
int8_t twoHundredAndOne = twoHundred + 1 ; /* so greater than twoHundred, huh? */
if (twoHundred > twoHundredAndOne) {
	Serial.print("what's going on?");
}

the signed 8 bits integers range from -128 to 127 so twoHundredAndOne overflows and its actual value is -55. A warning when comparing these signed and unsigned number is a great help!

Back to the HardwareSerial source code, in HardwareSerial.cpp, at line 98, the following comparison worries the compiler:

if (i != buffer->tail)

buffer->tail is declared in a structure (HardwareSerial.cpp line 66) as follows:

volatile unsigned int tail;

and i as follows:

int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE;

i is of data type int but tail is of data type unsigned int. Since none of these numbers may be negative a clean way to fix this warning is to declare i as an unsigned int:

unsigned int i = (unsigned int)(buffer->head + 1) % SERIAL_BUFFER_SIZE;

In the same manner, the warning of the line 467 may be fixed declaring the integer i line 462 as follows:

unsigned int i = (_tx_buffer->head + 1) % SERIAL_BUFFER_SIZE;

Second group: the last 2 warnings are: “unused variable”. In this case, a variable is created but never used. It could mean that the programmer failed to to use the variable or he forgot to remove its declaration after deleting a part of code which contained the variable. At line 368, the variable current_config is declared but never used, you can safely remove this declaration.

The second “unused variable” is found at line 127:

else {
      unsigned char c = UDR0;
    };

Since a variable declared as above only exists in the else scope, it will never be used. The purpose of this statement is not to store the content of the register UDR0 but to read it. The Atmel ATmega datasheet says: “reading the UDRn I/O location changes the buffer read location”, this means that reading UDR0 will change the state of other registers and is mandatory to do it this way in order to match the coding requirements. One way to keep the compiler quiet is to explicitly specify that the variable “c” is unused. This may be done by replacing the line 127 with the following statement:

unsigned char __attribute__ ((unused)) c = UDR0;

HTH

One Comment

  1. Mickael says:

    EDIT:
    Thanks to Stephan, this post was corrected. The previous version of this post said that the last warning spotted here can be removed by deleting the related “else” block. A part of the e-mail from Stephan explains why this is wrong and how to fix it:
    reading from the UDR0 hardware
    register changes the state of the hardware. From the Atmel datasheet for
    the ATmega328P: “Reading the UDRn I/O location will change the state of
    the receive buffer FIFO and consequently the TXB8n, FEn, DORn and UPEn
    bits, which all are stored in the FIFO, will change.

    It may well be that this branch serves a vital purpose. I would suggest to
    change this code to

    else {
    unsigned char __attribute__ ((unused)) c = UDR0;
    };

    to mellow the compiler.

Leave a Reply

You must be logged in to post a comment.