I am using snprintf like this to avoid a buffer overrun:
char err_msg[32] = {0};
snprintf(err_msg, sizeof(err_msg) - 1, "[ ST_ENGINE_FAILED ]");
I added the -1 to reserve space for the null terminator in case the string is more than 32 bytes long.
Am I correct in my thinking?
Platform:
As others have said, you do not need the -1 in this case. If the array is fixed size, I would use strncpy instead. It was made for copying strings - sprintf was made for doing difficult formatting. However, if the size of the array is unknown or you are trying to determine how much storage is necessary for a formatted string. This is what I really like about the Standard specified version of snprintf:
char* get_error_message(char const *msg) {
int e = errno;
size_t needed = snprintf(NULL, 0, "%s: %s (%d)", msg, strerror(e), e);
char *buffer = malloc(needed+1);
if (buffer) sprintf(buffer, "%s: %s (%d)", msg, strerror(e), e);
return buffer;
}
Combine this feature with va_copy and you can create very safe formatted string operations.
sprintf(buffer, …, since you have reserved buffer of the exact right size. That would avoid this bug.You don't need the -1, as the reference states:
The functions snprintf() and vsnprintf() do not write more than size bytes (including the trailing '\0').
Note the "including the trailing '\0'" part
No need for -1. C99 snprintf always zero-terminates. Size argument specifies the size of output buffer including zero terminator. The code, thus, becomes
char err_msg[32];
int ret = snprintf(err_msg, sizeof err_msg, "[ ST_ENGINE_FAILED ]");
ret contains actual number of characters printed (excluding zero terminator).
However, do not confuse with Microsoft's _snprintf (pre-C99), which does not null-terminate, and, for that matter, has completely different behaviour (e.g. returning -1 instead of would-be printed length in case if buffer is not big enough). If using _snprintf, you should be using the same code as in your question.
According to snprintf(3):
The functions
snprintf()andvsnprintf()do not write more thansizebytes (including the trailing'\0').
For the example given, you should be doing this instead:
char err_msg[32];
strncpy(err_msg, "[ ST_ENGINE_FAILED ]", sizeof(err_msg));
err_msg[sizeof(err_msg) - 1] = '\0';
or even better:
char err_msg[32] = "[ ST_ENGINE_FAILED ]";
strncpy() is incorrect. "At most n bytes of src are copied." I've adjusted the example to fix the null-termination.strncpy() will never overwrite the last character in the buffer. You have to explicitly do err_msg[sizeof(err_msg)-1] = '\0'; to get termination.sizeof will return the number of bytes the datatype will use in memory, not the length of the string. E.g. sizeof(int) returns '4' bytes on a 32-bit system (well, depending on the implementation I guess). Since you use a constant in your array, you can happily pass that to the printf.
sizeof to the destination array, which is completely correct.
err_msg buffer capacity (to guarantee it will not write past its end).
snprintf?_snprintf(), which doesn't behave like the standardsnprintf()(I think it doesn't always nul-terminate the string).