>
> On Thu, 2017-04-27 at 15:20 -0500, Jonathon Jongsma 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 (1).doc'
> > instead. If that also failed, it would attempt 'file (2).doc', etc,
> > up
> > to 63.
> >
> > Resolves: rhbz#1410181
> > Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > ---
> > Changes since v1:
> > - change from "file.doc (1)" to "file (1).doc"
> > - retain debug output for other CreateFile errors
> >
> > vdagent/file_xfer.cpp | 50
> > ++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 44 insertions(+), 6 deletions(-)
> >
> > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> > index f763ed3..24eb00a 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,52 @@ 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);
> > + char *extension = strrchr(file_name, '.');
> > +
> > + for (attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
> > + char dest_filename[MAX_PATH];
> > + if (attempt == 0) {
> > + strcpy(dest_filename, file_name);
> > + } else {
> > + if (name_len + POSTFIX_LEN > MAX_PATH) {
> > + vd_printf("failed creating %ls. Postfix makes name
> > too long for the buffer.", file_path);
> > + return;
> > + }
> > +
> > + int basename_len = (extension != NULL) ? (extension -
> > file_name) : name_len;
> > + memcpy(dest_filename, file_name, basename_len);
> > + // append postfix
> > + int postfix_len = snprintf(dest_filename +
> > basename_len, POSTFIX_LEN, " (%d)", attempt);
> > + if (extension != NULL) {
> > + strcpy(dest_filename + basename_len + postfix_len,
> > extension);
> > + }
> > + }
> > + if((MultiByteToWideChar(CP_UTF8, 0, dest_filename, -1,
> missing space^
> > file_path + wlen, MAX_PATH - wlen)) == 0){
> and here ^
> > + vd_printf("failed converting file_name:%s to WideChar",
> > dest_filename);
> > + 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) {
> > + 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;
>
> looks good, ack
>
> Pavel
>
I would propose a version like
diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
index f763ed3..de1aea1 100644
--- a/vdagent/file_xfer.cpp
+++ b/vdagent/file_xfer.cpp
@@ -28,6 +28,8 @@
#endif // compiler specific definitions
#include <stdio.h>
+#include <spice/macros.h>
+
#include "file_xfer.h"
#include "as_user.h"
@@ -99,13 +101,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)"
+ char *extension = strrchr(file_name, '.');
+ if (!extension)
+ extension = strchr(file_name, 0);
+
+ for (int attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
+ char dest_filename[SPICE_N_ELEMENTS(file_name) + POSTFIX_LEN];
+ if (attempt == 0) {
+ strcpy(dest_filename, file_name);
+ } else {
+ sprintf(dest_filename, "%.*s (%d)%s", int(extension - file_name), file_name,
+ attempt, extension);
+ }
+ if ((MultiByteToWideChar(CP_UTF8, 0, dest_filename, -1, file_path + wlen, MAX_PATH - wlen)) == 0) {
+ vd_printf("failed converting file_name:%s to WideChar", dest_filename);
+ 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) {
+ 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());
+ vd_printf("Failed creating %ls. More than 63 copies exist?", file_path);
return;
}
task = new FileXferTask(handle, file_size, file_path);
is more similar to the way is proposed in Unix version of the agent
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel