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

[Uri Lublin]

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:

diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index c456bbe..7249b21 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -93,8 +93,9 @@ void 
FileXfer::handle_start(VDAgentFileXferStartMessage* start,

      wlen = _tcslen(file_path);
      // make sure we have enough space
-    // (1 char for separator, 1 char for filename and 1 char for NUL 
terminator)
-    if (wlen + 3 >= MAX_PATH) {
+    // 1 = char for directory separator: "\"
+    const size_t POSTFIX_LEN = 6; // up to 2 digits in parentheses and 
final NUL: " (xx)"
+    if (wlen + 1 + POSTFIX_LEN >= MAX_PATH) {
          vd_printf("error: file too long %ls\\%s", file_path, file_name);
          return;
      }

Uri.