This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

The 'sscanf' mistake

sscanf(buffer, "%d", &var1_1byte);

in the above case, var1_1byte is uint8_t (ie 1 byte char variable).
the "%d" in sscanf resulted to writing of multiple bytes (notice that '%d' is for 'int') which inturn cleared the adjacent bytes of ram at address &var1_1byte.

thus, var1_1byte, var2_1byte got modified. checking the .map file i confirmed the error and realized the mistake.

i used '%d' instead of '%c' so that ascii value in buffer is converted to integer (actually wanted to convert to hex).

Ignorant. (Sic).

luckily i could trace the error in not more than 5 minutes. a newbie may have consumed more time hence thought of bringing to the notice of others.

  • I've never even considered using sscanf in an embedded (non-Linux) project. Always preferred to roll my own.

  • note:
    var1_1byte was at ram address -

    var1_1byte - 0x1000042c
    random_var - 0x1000042d
    var2_1byte - 0x1000042e

    the address can be checked by reading *.map file.

    ...ascii value in buffer is converted to integer (actually wanted to convert to hex)
    hex or integer is of less concern as the value will always be less than 9.

  • atoi can be used for conversion to integer.

    even considered using sscanf
    sscanf is a good function for conversion to float.

  • "Always preferred to roll my own"

    So what's your preferred approach?

    Maybe a State Machine parser?

  • I normally only use the scanf family for utility code. Maybe converting between two file formats.

    For production code, I normally write a real parser that includes range checking etc and can tell a user about found errors and on what line and column.

    Too many compilers do not support warnings/errors where the formatting characters doesn't match the data type which means even small code changes can introduce quite serious errors unless third-party code analyzer tools are used.

    With scanf() and some compilers, theres inttypes.h that has lots of macros for scanning different data types. But it really isn't so fun to have a huge amount of SCNi32, SCNiFAST16, SCNuLEAST16 etc in that formatting string depending on what data types that are used.

  • ... just as i would never use strcpy. I might, however use strncpy WITH THE ASSURANCE THERE WILL BE NO BUFFER OVERFLOW. ANY string routine that can overflow a buffer if something goes awry is asking for trouble. I have, on, at least one occasion, modified someone elsea code from strcpy to strncpy etc and things have improved, because runtime errors were caused that revealed other problems.

  • If using C and having functions

    bool get_u8(uint8_t *n,uint8_t min,uint8_t max);
    bool get_u16(uint16_t *n,uint16_t min,uint16_t max);
    bool get_u32(uint32_t *n,uint32_t min,uint32_t max);
    ...
    

    and then

    enum {
        PID_MIN = 1,
        PID_MAX = 999
    };
    typedef uint16_t pid_t;
    
    ...
    
    pid_t pid;
    if (!get_u16(&pid,PID_MIN,PID_MAX)) { ... }
    


    If the pid_t type is changed to a 32-bit type, then the compiler will give a compilation error because of incompatible pointer type to the get_u16() call - so the developer gets notified that the input code needs to be adjusted to match the new data type.

    With C++, the compiler could even automatically switch to the correct function with:

    bool get_value(uint8_t &n,uint8_t min,uint8_t max);
    bool get_value(uint16_t &n,uint16_t min,uint16_t max);
    bool get_value(uint32_t &n,uint32_t min,uint32_t max);
    

    With scanf(), a large number of compilers will just silently accept an incorrect formatting string, resulting in memory overwrites which might not be easy to spot depending on big- or little-endian processors.

    The large problems with incorrect formatting strings is one of the reasons why C++ got streams with overloaded << and >> to make the source code type-safe.

    Note that strncpy() and strncat() are not so glorious as they might initially look to be. They do not create any memory overwrites, but also doesn't guarantee a zero-terminated buffer. And they do not tell if the buffer was too short so other functions then making use of that string are likely to read past the buffer end.

    The problem with the missing termination is that strlen() can't be used to check for overflow. And dest[sizeof(dest)-1] != '\0' can't be used because that position has undefined content if the concatenated string was shorter. So strncpy() and strncat() then requires strnlen() (which is non-standard) or memchr() to check for overflow.

    It can sometimes be better to "print" the copy or concatenation:

    if (snprintf(dest,sizeof(dest),"%s%s",s1,s2) >= sizeof(dest)) {
        ouch_buffer_too_small();
    }
    

  • luckily i could trace the error in not more than 5 minutes.
    You appear somewhat proud of that achievement. Don't be. Any coding style which allows this to consume as much as 5 seconds has to be considered fatally flawed, IMHO.

    This type of silly coding mistake is what compiler warnings exist to tell you about. If you weren't warned by your compiler about this construct long before it was ever executed, you have the warning level set considerably too far down for your own good.

    a newbie may have consumed more time hence thought of bringing to the notice of others.

    Which is why newbies should always have their compilers' warning level cranked way up high.

  • So what's your preferred approach?

    Specific functions for specific data types.

    I've yet to find a non-(Linux or similar) situation where I need to be able to interpret every data format supported by sscanf. And if I did, I'd then question the suitability of the chosen platform.

  • Which is why newbies should always have their compilers' warning level cranked way up high.

    Agree wholeheartedly with that. But would not just limit it to newbies. Seasoned professionals can have off moments too. An occasional warning can help keep one alert.