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

[Frediano Ziglio]

> 
> 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)".

> 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