Arduino compiler warnings

Hi all,

I just had an interesting issue that took a little poking to resolve.

The sketch I was working with received two bytes from the serial port and then sent quite a few back, depending on what was received.

I declared some variables:

byte rx_buffer[25];
byte rx_pos;
byte packet_len;

followed by a few more.

Not long into the run of my program things went horribly wrong, and a bit of debugging showed that the variables after the buffer were being corrupted.

I inserted a temporary buffer in between my rx_buffer and the rest of my variables and zeroed it in setup and added code to tell me which positions in the buffer were non-zero.

The buffer was being written to two bytes at a time after some point, and some playing with the length showed it didn’t corrupt past position 103.

Anyhow, I found what was happening. The file wiring_serial.c has a global variable:

byte rx_buffer[128];

And so everything got confused.

So I can think of three fixes that could prevent people being burned by stuff like this in the future - in order of difficulty:

  1. Add all the global variables to the Keywords page.

  2. Turn on some compiler warnings. This might require a bit of parsing and suppressing of warnings that might not be too beginner friendly, but variable redeclaration is probably something that needs to be caught.

  3. Either manually or using clang or antlr or something, write a utility that scans the c files in /hardware/arduino/cores and prepends some kind of namespace prefix to the global variables, both for their declaration and internal usage. If this sounds like it’s the best option and I get some free time in the coming months I might do this just for kicks.

I just thought I’d let people know there was a potential pitful lurking around and waiting to pounce on beginner programmers.

Cheers

Maybe file a bug report against wiring_serial?

This is [u]exactly[/u] the reason variables should [u]not[/u] be global. At most it should be global to that section of code. If it's written in C++ it could be made private.

-j

I agree completely (although private wouldn't work in this case because rx_buffer is not a member of any class). A very easy fix is to simply use the static keyword as in:

static unsigned char rx_buffer[128];
static int rx_buffer_head = 0;
static int rx_buffer_tail = 0;

When used like this, static indicates that the variables have "internal linkage", which means they are not visible outside the source module that contains them.

Another C++-y alternative would be to use a namespace, which is designed specifically for avoiding this kind of collision. But static is easier.

How do you file a bug report anyway -- or did we just do that?

M