Pages: [1]   Go Down
Author Topic: Error message buffer overflow.  (Read 515 times)
0 Members and 1 Guest are viewing this topic.
UK
Offline Offline
Full Member
***
Karma: 0
Posts: 125
The red wire is ground, right ???
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi,

I use a character buffer to hold and display error messages. Very occasionally it overflows, when this happens I want to flag this by overwriting the last few characters of the error message with the word 'overflow'. As follows.....
Code:
  if (strlen(output) > gBuffSize)
  {
    // Message has overflown the buffer! data may have been corrupted!
    output[gBuffSize-9] = 'O';
    output[gBuffSize-8] = 'V';
    output[gBuffSize-7] = 'E';
    output[gBuffSize-6] = 'R';
    output[gBuffSize-5] = 'F';
    output[gBuffSize-4] = 'L';
    output[gBuffSize-3] = 'O';
    output[gBuffSize-2] = 'W';
    output[gBuffSize-1] = '/n';
  }
This works, but I can't help feeling that there's got to be a better solution - something a bit more elegant. Any suggestions?

Cheers
Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 239
Posts: 24371
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I can't help feeling that there's got to be a better solution - something a bit more elegant. Any suggestions?
Well, call me crazy, but I wouldn't allow it to overflow in the first place.
Pragmatic, rather than elegant.
Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

UK
Offline Offline
Full Member
***
Karma: 0
Posts: 125
The red wire is ground, right ???
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I wouldn't allow it to overflow in the first place.
- if I was that good, I wouldn't have any errors in any of my code and wouldn't need anything to handle error messages!  smiley-wink

Seriously, you're right, the error shouldn't be allowed in the 1st place, but, the error message is frequently generated from a combination of strings and numbers from multiple sources, that include (for example) error messages from GSM modems. I first noticed this issue when a GSM module generated the longer than planned for error "Roaming not allowed in this location area" which busted the buffer and corrupted something elsewhere.
The error handling code is in a singleton logger used throughout my projects, so the quickest fix is insert the word 'overflow' and then, once the error has been spotted, trace it back to the source and fix it - a lot simpler than checking the length of each clause of the error message before it gets displayed.

Cheers
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 549
Posts: 46113
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Once the overflow occurs, any attempt to handle it is not guaranteed to succeed.

You need to post all of your code, for a peer review.
Logged

UK
Offline Offline
Shannon Member
****
Karma: 184
Posts: 11179
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

a lot simpler than checking the length of each clause of the error message before it gets displayed.

I don't agree that it's simpler. You should be formatting your error messages for display using a common algorithm, and that should check for and avoid buffer overflow. Anywhere you are dealing with variable length data in fixed length buffers you should code it to avoid the possibility of buffer overflow as a matter of course - explicitly testing for and trying to report buffer overflow after the event is a very poor solution which in no way addresses the underlying problem.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

UK
Offline Offline
Full Member
***
Karma: 0
Posts: 125
The red wire is ground, right ???
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Once the overflow occurs, any attempt to handle it is not guaranteed to succeed.
True, but any attempt is better than no attempt.

Quote
You need to post all of your code
Probably best if I filter it a bit, so here's a further example......

Here's the basic error message, defined in progmem to save memory....
Code:
const prog_char ACC_ERR_977[]    PROGMEM = "W - So far, <%ld> good leptons, and <%ld> pseudo gauge bosons.";
 
and here's where I use it to create a full error message for logging....
Code:
sprintf_P(_messBuff,ACC_ERR_977,_leptons,_pgBoson);
logging(_messBuff);

The logging() function then displays it on the screen, and/or writes to SD card.

If _messBuff is say 65 characters, then this works fine until the length of _leptons and _pgBoson exceeds a total of about 8 characters, at which point _messBuff gets busted.

Another example, this from a GPS project....
Code:
const prog_char GPS_ERR_366[]    PROGMEM = "E - the latitude contains non-numeric values <%s>";
sprintf_P(_messBuff,GPS_ERR_366,_gpsBuff);
logging(_messBuff);

_gpsBuff is about 80 characters (normal maximum NMEA sentence length) but should normally only be holding 9 characters when processing latitude, however, if the GPS module has decided to generate some weird sh1t which it does occasionally the length of the PROGMEM string plus the length of whatevers in the gps buffer may exceed the length of _messBuffer.

Since the overflow issue first occurred I've done what I can to check the length of all the combined elements of the message before actually creating it, but I've used this technique in literally 100s of places so I added the snippet in my original posting as an extra safety net whilst I get the underlying issues fixed.

I guess what I'm really looking for is a function that will tell me how long the string created by sprintf_P() will be before I actually use it !

Cheers




Logged

UK
Offline Offline
Shannon Member
****
Karma: 184
Posts: 11179
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I guess what I'm really looking for is a function that will tell me how long the string created by sprintf_P() will be before I actually use it !

Yes, that seems to be what you're looking for. Happily, snprintf_P() and friends does exactly what you need.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 549
Posts: 46113
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
I guess what I'm really looking for is a function that will tell me how long the string created by sprintf_P() will be before I actually use it !
You mean before you create it? If you supply a buffer that is too small, by the time sprintf_P tells you how many bytes (and it does) it has written to the buffer, it has already overflowed.
Logged

UK
Offline Offline
Full Member
***
Karma: 0
Posts: 125
The red wire is ground, right ???
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Happily, snprintf_P() and friends does exactly what you need.

Thanks - just what I was looking for.

Quote
You mean before you create it?
Yes, I meant 'create' not 'use.


Thanks for your help everyone
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Code:
  if (strlen(output) > gBuffSize)
  {
    // Message has overflown the buffer! data may have been corrupted!
    output[gBuffSize-9] = 'O';
    output[gBuffSize-8] = 'V';
    output[gBuffSize-7] = 'E';
    output[gBuffSize-6] = 'R';
    output[gBuffSize-5] = 'F';
    output[gBuffSize-4] = 'L';
    output[gBuffSize-3] = 'O';
    output[gBuffSize-2] = 'W';
    output[gBuffSize-1] = '/n';
  }

Code:
memcpy (&output[gBuffSize-9], "OVERFLOW\n", 9);


Note it is \n not /n.

Also you might want to allow for the trailing 0x00 byte.
Logged

Pages: [1]   Go Up
Jump to: