sprintf with M0 and C++

[mlewus]

I got hung up with some of the sample code in the Mailbox Notifier. It was designed to run on the ATMEGA but hangs when complied and run on a Moteino M0.

The .ino uses several instances of sprintf().  The code looks like a variation on this:

Code Select Expand

char* MLOstr="LO:99d23h59m";
unsigned long lastOpened;
//... some code to assign a value to lastOpened...
long LC = lastOpened;
char periodO;
//... some code that assigns value to periodO... 

if (periodO == 'd')
      sprintf(MLOstr, "LO:%ldd%ldh", lastOpened, (LO%86400)/3600);

If you run this code on the M0 using any recent version of Arduino, when run, the code will hang at the sprintf statement. It runs fine on the ATMEGA version. 

This seems to me to be a bug because the declaration

Code Select Expand

char* MLOstr = "LO:99d23h59m";

in C (at least) creates only a pointer and does not allocate any memory for the array itself. It is presumed that the array memory was defined elsewhere, which is why the compiler does not complain.

An alternate declaration that allocates memory will work with no other code changes:

Code Select Expand

char MLOstr[13] = "LO:99d23h59m";

My question is, does anyone know why this works on the original ATMega version of the code, but only crashes on the M0 version? Is this a C vs C++ thing, or is it what they mean when they say "behavior is undefined"?

[TomWS]

The first declaration is flawed if the variable is intended to be a target for storing new values (as with being a target of an sprintf). That it works on ATMEGA is just one more example of the sloppy work done to make Arduino 'easy' to use.  Literally the declaration is creating a char pointer to point to a constant character literal.  The sprintf statement is treating the variable as a modifiable character buffer, which it isn't.  It should fail.

Your third declaration creates a real character buffer that is initialized to the constant character literal, but IS modifiable.  This is correct.

The declaration:

Code Select Expand

char MLOstr[] = "LO:99d23h59m";

will also work, assuming that the sprintf statement never produces an output that is greater than 12 characters long.

The real problem isn't the M0 compiler, it is the original compiler for the ATMEGA not flagging this as an error.

Tom

[Felix]

mlewus,
I think the intent is to have a buffer that is modifiable, and is declared with a dummy content that should represent be the longest ever string stored in that buffer.
I would normally declare it as a predefined length or prefilled buffer instead, as seen in this sketch:

Code Select Expand

char MLOstr[20];

Was your code grabbed from somewhere on LowPowerLab.com or github repos?

[mlewus]

Felix,

The code in question came from https://github.com/LowPowerLab/MailboxNotifier.

The following sketches have similar declarations that do not reserve memory:
MailboxNotifier.ino
MailboxNotifier2_sender.ino
MailboxNotifier3_sender.ino
MailboxNotifier4_sender_withWeatherShield.ino

The declarations are the same in all sketches above look lke this:

Code Select Expand

    char* MLOstr="LO:99d23h59m";
    char* MLCstr="LC:99d23h59m";
    char* BATstr="BAT:1024";

However, the mailbox notifier code in RFM69/Examples (https://github.com/LowPowerLab/RFM69/tree/master/Examples/MailboxNotifier) is correct:

Code Select Expand

char BATstr[20];
char MLOstr[20];

Thanks for all of your tremendous hard work in making these libraries and examples available to us!

[Felix]

Thanks for finding and reporting these
I fixed those sketches to make them into proper char buffers.
If you find more, please let me know!
Thanks