[Spice-devel] [PATCH win-vdagent] Support file transfer for existing files

[Christophe de Dinechin]

> On 27 Apr 2017, at 17:31, Pavel Grunt <pgrunt at redhat.com <mailto:pgrunt at redhat.com>> wrote:
> 
> On Thu, 2017-04-27 at 10:25 -0500, Jonathon Jongsma wrote:
>> On Wed, 2017-04-26 at 17:50 -0400, Frediano Ziglio wrote:
>>>> 
>>>> Previously, if the user attempted to transfer a file that had
>>>> the
>>>> same
>>>> name as a file that already exists on the guest, we would just
>>>> fail
>>>> the
>>>> transfer. This patch tries to match the behavior of the linux
>>>> vdagent
>>>> where we try to append an integer onto the name of the new file
>>>> to
>>>> make
>>>> it unique. For example, if you tried to transfer 'file.doc' to
>>>> the
>>>> guest
>>>> and that file already existed, it would try to create 'file.doc
>>>> (1)'
>>>> instead. If that also failed, it would attempt 'file.doc (2)',
>>>> etc,
>>>> up
>>>> to 63.
>>>> 
>>> 
>>> Not really user friendly, Windows is really extension based.
>>> Would be better to be 'file (1).doc' for 'file.doc'.
>>> Yes, not easy to implement as adding " (xx)".
>> 
>> True, I just tried to copy the behavior of the linux vdagent (as
>> suggested in the bug), but perhaps we should do it a bit differently
>> for windows. Or maybe we should change both the linux and vdagent to
>> work this way...
> 
> Probably. I checked Nautilus and Thunar (file managers in Gnome and
> Xfce) behavior when copying a same file in the same folder and they
> are adding (xxx) before the extension suffix.

You can clone / adapt this code from glib:

https://github.com/GNOME/glib/blob/master/gio/glocalfile.c#L1727 <https://github.com/GNOME/glib/blob/master/gio/glocalfile.c#L1727>

I personally like the “%.*s” ;-), it’s a smart way to cut the string. They generate something like foo.1.c and not foo(1).c, but I’m sure you can fix that :-)

Christophe

> 
> Pavel
> 
>> 
>>> 
>>>> Resolves: rhbz#1410181
>>>> ---
>>>>  vdagent/file_xfer.cpp | 38 ++++++++++++++++++++++++++++++++--
>>>> ----
>>>>  1 file changed, 32 insertions(+), 6 deletions(-)
>>>> 
>>>> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
>>>> index f763ed3..14e2098 100644
>>>> --- a/vdagent/file_xfer.cpp
>>>> +++ b/vdagent/file_xfer.cpp
>>>> @@ -61,6 +61,7 @@ void
>>>> FileXfer::handle_start(VDAgentFileXferStartMessage*
>>>> start,
>>>>      HANDLE handle;
>>>>      AsUser as_user;
>>>>      int wlen;
>>>> +    int attempt = 0;
>>>>  
>>>>      status->id = start->id;
>>>>      status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
>>>> @@ -99,15 +100,40 @@ void
>>>> FileXfer::handle_start(VDAgentFileXferStartMessage*
>>>> start,
>>>>  
>>>>      file_path[wlen++] = TEXT('\\');
>>>>      file_path[wlen] = TEXT('\0');
>>>> -    if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_name, -1,
>>>> file_path +
>>>> wlen, MAX_PATH - wlen)) == 0){
>>>> -        vd_printf("failed converting file_name:%s to WideChar",
>>>> file_name);
>>>> -        return;
>>>> +
>>>> +    const int MAX_ATTEMPTS = 64; // matches behavior of linux
>>>> vdagent
>>>> +    const size_t POSTFIX_LEN = 6; // up to 2 digits in
>>>> parentheses
>>>> and final
>>>> NULL: " (xx)"
>>>> +    size_t name_len = strlen(file_name);
>>>> +    for (attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
>>>> +        char *pos = &file_name[name_len];
>>>> +        if (attempt > 0) {
>>>> +            if (name_len + POSTFIX_LEN > MAX_PATH) {
>>>> +                vd_printf("failed creating %ls. Postfix makes
>>>> name
>>>> too long
>>>> for the buffer.", file_path);
>>>> +                return;
>>>> +            }
>>>> +            snprintf(pos, POSTFIX_LEN, " (%d)", attempt);
>>>> +        }
>>>> +        if((MultiByteToWideChar(CP_UTF8, 0, file_name, -1,
>>>> file_path + wlen,
>>>> MAX_PATH - wlen)) == 0){
>>>> +            vd_printf("failed converting file_name:%s to
>>>> WideChar",
>>>> file_name);
>>>> +            return;
>>>> +        }
>>>> +        handle = CreateFile(file_path, GENERIC_WRITE, 0, NULL,
>>>> CREATE_NEW,
>>>> 0, NULL);
>>>> +        if (handle != INVALID_HANDLE_VALUE) {
>>>> +            break;
>>>> +        }
>>>> +
>>>> +        // If the file already exists, we can re-try with a new
>>>> filename. If
>>>> +        // it's a different error, there's not much we can do.
>>>> +        if (GetLastError() != ERROR_FILE_EXISTS) {
>>> 
>>> I would include the old line:
>>> 
>>>    vd_printf("failed creating %ls %lu", file_path,
>>> GetLastError());
>>> 
>>>> +            return;
>>>> +        }
>>>>      }
>>>> -    handle = CreateFile(file_path, GENERIC_WRITE, 0, NULL,
>>>> CREATE_NEW, 0,
>>>> NULL);
>>>> -    if (handle == INVALID_HANDLE_VALUE) {
>>>> -        vd_printf("failed creating %ls %lu", file_path,
>>>> GetLastError());
>>>> +
>>>> +    if (attempt == MAX_ATTEMPTS) {
>>>> +        vd_printf("Failed creating %ls. More than 63 copies
>>>> exist?",
>>>> file_path);
>>>>          return;
>>>>      }
>>>> +
>>>>      task = new FileXferTask(handle, file_size, file_path);
>>>>      _tasks[start->id] = task;
>>>>      status->result = VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA;
>>> 
>>> Frediano
>> 
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org <mailto:Spice-devel at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org <mailto:Spice-devel at lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/spice-devel <https://lists.freedesktop.org/mailman/listinfo/spice-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170428/2b629440/attachment-0001.html>