Strange variable corruption

[v8dave]

My code has been running perfectly until I added the following code to it today.

TimeDelay = NetworkWaitTime[NetworkWaitPtr];
sprintf(TxtBuffer, "Waiting %d seconds to retry", (int) TimeDelay);

NetworkWaitTime and NetworkWaitPtr are declared globally to the function as follows:

int NetworkWaitTime[7] = { 60, 60, 120, 300, 300, 600, 900 };
int NetworkWaitPtr;

NetworkWaitPtr is initialised to 0 during the thread initialisation.

If I set a breakpoint at the TimeDelay= line I can step past this and TimeDelay = 60 as expected. If I then step the sprintf line it works and the TxtBuffer has the text as shown along with the count of 60 in it. BUT, NetworkWaitPtr is now showing a value of 36327262. If I then try to run the code, it crashes with a memory access fault. TxtBuffer is declared locally and is [30] in size. Plenty of room for the string.

Something is overwriting or currupting the variable and I can't see where other than the possibility that sprintf is not thread safe, ie, not re-entrant? Seems strange though as I am using this elsewhere with no effect on the code.

Regards
Dave...

[Ridgeglider]

what variables, arrays, etc are decalared near NetworkWaitPtr?It sounds like a buffer overflow or that you are writing a variable that is too large into an adjoining location. Take a look at how you declared TxtBuffer.... When it gets loaded by sprintf, I'll bet you are clobbering NetworkWaitPtr nearby. Consider using snprintf so that you cant overun the space allotted in TxtBuffer. Why do you cast TimeDelay as an (int) in the sprintf buffer; it's already declared as an int.

[v8dave]

Hi,

TxtBuffer is declared local to the function. NetworkWaitPtr is global to the module.

I setup memset to clear TxtBuffer on entry to the function and after the sprintf call, it only contained the text and the rest, some 12 bytes was NULL.

With these 2 lines commented out, the rest of the code runs fine with no issues and NetworkWaitPtr never gets corrupted.

Need to do more digging.

Dave...

[Ridgeglider]

Is TxtBuffer trully big enough to hold chars and delimiter? Try making it much bigger, and also limit what sprintf can write to it by using snprintf instead.

[v8dave]

Yes. The string being written to it is only 27 bytes and I declared 40 bytes.

I'll try increasing this and see what effect it has.

Dave...

[tod]

Of course you could avoid worries about buffer sizes by using the C++ sstream library. It also might make for an interesting test.

Code: Select all

#include <sstream>
#include <iostream>

...
using namespace std;
...

TimeDelay = NetworkWaitTime[NetworkWaitPtr];
ostringstream msg_buf;
msg_buf << "Waiting " << (int)TimeDelay << "seconds to retry"<< endl;
//Use msg_buf.str() to get back a c++ string or msg_buf.str().c_str() to get a c string

CLUMP-CLUMP (the sound of me getting off my C++ soapbox)

[rnixon]

The problem could be that there is a memory overwrite problem somewhere else in your code. It only becomes apparent when you add these few lines because the move things around in memory so that the corruption becomes a problem. Especially when using variable parameter functions that make heavy use of the stack, or areas beyond the stack if the stack is too small

[v8dave]

rnixon was right. It was somewhere else in the code. I have defined a char array to hold the Date as 10 bytes and then made a change to show a 4 digit year instead but without changing the array size. Now it was writing to the 11th byte!! Oops.

All is now well.

Dave...

[tod]

(clump clump - going up)So here are some rhetorical questions for you. For the problem you found if you had used sstream library would the problem have been avoided? When you fixed it did you use sstream or just increase the size of the array? If the latter, how will you avoid a similar problem in the future (not necessarily with this method but similar ones)? (clump clump - getting down)

[rnixon]

I have to admit I have not used the sstream libs, but I will check them out since you make some good points. I try to use only static allocations in embedded systems.

Accidents can always happen, no matter how careful you are or what methods you use. Memory leaks are just one issue. So my answer to you question would be from an architecture point of view. If you organize structured code in C, or objects in C++, and have error checking on lengths and bad values, it would avoid this problem as well as many others. For example, if you have a function or object that prints dates, hopefully it would be obvious what is going on and what the parameters are.

My own preference is to keep the C++ at a minimum. I mostly use classes, which my brain just interprets as C structures with functions. Everything needs to be intuitive and the code needs to tell the story. If you overload a common operator in C++, your fired, because the next coder can't easily tell what is going on (I'm not on your soapbox, just standing next to it).