converting sprintf to snprintf using MACRO

Question

We have some code like following:

#define MAXINT  1000000000

#define SPRINTF(a, b, c)    ((sizeof (a) > 8) ? snprintf(a, sizeof (a), b, c) : snprintf(a, MAXINT, b, c))
                            // avoids buffer overrun for static buffers of size > 8 else behaves like default sprintf()
                            // note max. size of char pointer is 8, so if sizeof (a) > 8, it means it is static array

I remember the developer was asked to convert all sprintf() to safer version snprintf() and he did change as shown above.

I understand that above MACRO avoids buffer corruption ONLY for static buffers of size > 8 and for other i.e. dynamic buffers and static buffers of size <=8 behaves like a normal sprintf() assuming that the string being copied is not greater than "MAXINT". Is this correct?

For ex. if string being copied is less than dest buffer or greater than dest buffer, this statement/above MACRO will always behave correctly either like snprintf(a, sizeof(a), b, c) or like normal default sprintf() - which is kind of OK behaviour for the time being. I assume it will never fill the dest buffer with MAXINT (too big and src string will never be that big) amount of characters?

That macro is horrible. It gives a coder false sense of security. I pity the poor soul who has to find some weird bug a few years from now. I would really recommend a doing it the hard way: get rid of the macro and fix those sprintf manually. — user694733
Don't do that. Use a tool to automatically refactor to snprintf when it can prove the buffer is on the stack, and manually for heap buffers. Furthermore, that macro is BAD. Instead of hardcoding 8, you should use sizeof(void*), and instead of a fake MAX_INT, use sprintf, it's the same behavior without a useless comparison. — toasted_flakes

Akhi Akhi · Accepted Answer · 2014-06-09T07:49:27

The macro has the conditional 'a > 8' to check if a is of type char* or char[]. If char*, there is no way of knowing how much memory was allocated to it and how much we can write to it. If char[], then we do know how much memory is reserved for it. So for char[], we can use snprintf and thereby be safer by ensuring we never overrun. For char*, we cannot offer the same. The code is assuming that you will never want to write more than MAXINT.

If I was writing the macro, I would have changed snprintf(a, MAXINT, b, c) to sprintf(a, b, c) to make it clearer that we are unable to offer any guarantees in that scenario.

converting sprintf to snprintf using MACRO

1 Answers