[Spice-devel] [vdagent-win PATCH] file-xfer: handle_start: use snprintf instead of sprintf_s

[Uri Lublin]

On 3/3/19 12:26 PM, Uri Lublin wrote:
> On 2/28/19 3:12 PM, Frediano Ziglio wrote:
>>> On 2/25/19 4:12 PM, Frediano Ziglio wrote:
>>>>>
>>>>> When building with older mingw, sprintf_s does not
>>>>> always work as expected, but snprintf does.
>>>>>
>>>>> Also it's more consistent in the file.
>>>>>
>>>>> Note that when building with VS, snprintf becomes sprintf_s
>>>>>
>>>>
>>>> I think this could be a bug, from documentation:
>>>>
>>>> "If the buffer is too small for the formatted text, including the
>>>> terminating null, then the buffer is set to an empty string by 
>>>> placing a
>>>> null character at buffer[0], and the invalid parameter handler is 
>>>> invoked"
>>>>
>>>> which is different from snprintf behaviour, _snprintf is probably 
>>>> what do
>>>> we want.
>>>
>>> Actually, I think we do want snprintf's behavior, not _snprintf.
>>> _snprintf does not guarantee null termination of the string.
>>>
>>
>> Yes, you are right, sorry for the confusion.
>>
>>> We should probably check the return value.
>>
>> Well, yes, and in call to snprintf, even on Linux. Or continue to
>> ignore as we do.
>>
>>> Also we can add a check that there is enough space and fail early.
>>>
>>
>> This does not make sense, better to check later, it's easier.
>> But usually you just call snprintf and ignore if truncated,
>> I don't see why in this case we should make an exception.
> 
> 
> There is a check already with +3.
> We can make it with +1+6, so we know there is enough space:

On second thought, that's not good.
1. The filename length is not in the calculation
2. It's possible that the file is long and can
    be created, but with the (xx) addition it's too long.

Uri