[MPlayer-dev-eng] [PATCH 6/7] stream ftp: Allocate command buffer on-heap

[Hans-Dieter Kosch]

Reimar Döffinger wrote:
> 
> On 15 Nov 2012, at 03:42, Sergey <sergemp at mail.ru> wrote:
> 
>> On Mon, 12 Nov 2012 Reimar Döffinger wrote:
>>
>>>> +    snprintf(p->cmd_buf,CMD_BUFSIZE - 1,"REST %"PRId64, (int64_t)newpos);
>>> There should be no -1 for the size argument (same for all other
>>> snprintfs).
>> Are you sure?
>>
>> My man snprintf says:
>>    The  functions  snprintf() and vsnprintf() write at most size
>>    bytes (including the trailing null byte (’\0’)) to str.
>> But it does not say that \0 is always stored there.
> 
> Read the POSIX or C99 specification.
> 
>> Also this page explicitly says that it may not be stored:
>>  http://msdn.microsoft.com/en-us/library/2ts7cx93.aspx
> 
> Microsoft does not have a snprintf function at all, you are referring to _snprintf here.
> Also subtracting 1 would still not help one bit with that one, since you would still miss the 0-termination.

Reimar is right.

The man page does not explicitly state that '\0' is written and counted, but 
from the way how it is expressed, one can clearly deduct that it's true, and it 
*is*. There are myriads of such phrasings in man pages. It's the peculiar style 
of UNIX man pages, expressed as short as possible. One cannot pick it to pieces 
like a lawyer.

Don't confuse snprintf() with strncpy(), which in deed doesn't write '\0' on 
overflow; the behaviour of the latter is however clearly stated in the man page.


Without digging in the source, snprintf() can easily be verified with a simple 
test program:

#include <stdio.h>
main()
{
     char s[] = "aaaa";
     printf("%s\n", s);
     snprintf(s, 3, "%s", "bbbb");
     printf("%s\n", s);
}

The output is:
aaaa
bb

If '\0' were not written, the output were:
aaaa
bbba


Conclusion:
-1 is definitely incorrect. Otherwise, as Reimar said, it would be mandatory to 
explicitly terminate the truncated string with '\0' (like with strncpy() usage).