Re: [PATCH win-vdagent] Support file transfer for existing files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 27 Apr 2017, at 17:31, Pavel Grunt <pgrunt@xxxxxxxxxx> 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:


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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]