Error message buffer overflow.

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.....

  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

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.

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! :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

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.

BigusDickus:
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.

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

True, but any attempt is better than no attempt.

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....

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....

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....

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

BigusDickus:
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.

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.

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

Thanks - just what I was looking for.

You mean before you create it?

Yes, I meant 'create' not 'use.

Thanks for your help everyone

  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';
  }

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

Note it is \n not /n.

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