[Spice-devel] [PATCH] vdagent-win: Use wide characters in drap&drop code

[Frediano Ziglio]

> 
> On Thu, Apr 23, 2015 at 08:23:18AM -0400, Frediano Ziglio wrote:
> > This allow username to contain any extended characters.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  vdagent/file_xfer.cpp | 32 +++++++++++++-------------------
> >  vdagent/file_xfer.h   |  9 +++++----
> >  2 files changed, 18 insertions(+), 23 deletions(-)
> > 
> > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> > index 2dc138b..275768b 100644
> > --- a/vdagent/file_xfer.cpp
> > +++ b/vdagent/file_xfer.cpp
> > @@ -39,7 +39,7 @@ FileXfer::~FileXfer()
> >      for (iter = _tasks.begin(); iter != _tasks.end(); iter++) {
> >          task = iter->second;
> >          CloseHandle(task->handle);
> > -        DeleteFileA(task->name);
> > +        DeleteFile(task->name);
> >          delete task;
> >      }
> >  }
> > @@ -48,7 +48,8 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> > start,
> >                              VDAgentFileXferStatusMessage* status)
> >  {
> >      char* file_meta = (char*)start->data;
> > -    char file_path[MAX_PATH], file_name[MAX_PATH];
> > +    TCHAR file_path[MAX_PATH];
> > +    char file_name[MAX_PATH];
> >      ULARGE_INTEGER free_bytes;
> >      FileXferTask* task;
> >      uint64_t file_size;
> > @@ -73,12 +74,12 @@ void
> > FileXfer::handle_start(VDAgentFileXferStartMessage* start,
> >          return;
> >      }
> >  
> > -    if (FAILED(SHGetFolderPathA(NULL, CSIDL_DESKTOPDIRECTORY |
> > CSIDL_FLAG_CREATE, NULL,
> > +    if (FAILED(SHGetFolderPath(NULL, CSIDL_DESKTOPDIRECTORY |
> > CSIDL_FLAG_CREATE, NULL,
> >              SHGFP_TYPE_CURRENT, file_path))) {
> >          vd_printf("failed getting desktop path");
> >          return;
> >      }
> > -    if (!GetDiskFreeSpaceExA(file_path, &free_bytes, NULL, NULL)) {
> > +    if (!GetDiskFreeSpaceEx(file_path, &free_bytes, NULL, NULL)) {
> >          vd_printf("failed getting disk free space %lu", GetLastError());
> >          return;
> >      }
> > @@ -87,25 +88,18 @@ void
> > FileXfer::handle_start(VDAgentFileXferStartMessage* start,
> >          return;
> >      }
> >  
> > -    if (strlen(file_path) + strlen(file_name) + 1 >= MAX_PATH) {
> > -        vd_printf("error: file too long %s\\%s", file_path, file_name);
> > +    wlen = _tcslen(file_path);
> > +    if (wlen + 3 >= MAX_PATH) {
> 
> Why 3 ?
> 

On char for the separator, one for the file (can't be empty) and one for the NUL terminator.

> > +        vd_printf("error: file too long %ls\\%s", file_path, file_name);
> >          return;
> >      }
> >  
> > -    vdagent_strcat_s(file_path, sizeof(file_path), "\\");
> > -    vdagent_strcat_s(file_path, sizeof(file_path), file_name);
> > -    if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0)) ==
> > 0){
> > +    file_path[wlen++] = TEXT('\\');
> 
> I'd add file_path[wlen] = TEXT('\0') as we currrently are going to try
> to print that string if an error occurs.
> 

Agreed

> > +    if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_name, -1, file_path +
> > wlen, MAX_PATH - wlen)) == 0){
> >          vd_printf("failed getting WideChar length of %s", file_path);
> 
> Rather vd_printf("failed converting file_name:%s to WideChar", file_name);
> 

Agreed

> >          return;
> >      }
> > -    TCHAR *wfile_path = new TCHAR[wlen];
> > -    if (MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, wlen)
> > == 0){
> > -        vd_printf("failed converting file_path:%s to WindChar",
> > file_path);
> > -        delete[] wfile_path;
> > -        return;
> > -    }
> > -    handle = CreateFile(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
> > NULL);
> > -    delete[] wfile_path;
> > +    handle = CreateFile(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
> > NULL);
> >      if (handle == INVALID_HANDLE_VALUE) {
> >          vd_printf("failed creating %s %lu", file_path, GetLastError());
> >          return;
> > @@ -150,7 +144,7 @@ fin:
> >      if (task) {
> >          CloseHandle(task->handle);
> >          if (status->result != VD_AGENT_FILE_XFER_STATUS_SUCCESS) {
> > -            DeleteFileA(task->name);
> > +            DeleteFile(task->name);
> >          }
> >          _tasks.erase(iter);
> >          delete task;
> > @@ -176,7 +170,7 @@ void
> > FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
> >      }
> >      task = iter->second;
> >      CloseHandle(task->handle);
> > -    DeleteFileA(task->name);
> > +    DeleteFile(task->name);
> >      _tasks.erase(iter);
> >      delete task;
> >  }
> > diff --git a/vdagent/file_xfer.h b/vdagent/file_xfer.h
> > index 7ac911e..d9c65f3 100644
> > --- a/vdagent/file_xfer.h
> > +++ b/vdagent/file_xfer.h
> > @@ -22,17 +22,18 @@
> >  #include "vdcommon.h"
> >  
> >  typedef struct ALIGN_VC FileXferTask {
> > -    FileXferTask(HANDLE _handle, uint64_t _size, char* _name):
> > +    FileXferTask(HANDLE _handle, uint64_t _size, const TCHAR* _name):
> >      handle(_handle), size(_size), pos(0) {
> >          // FIXME: should raise an error if name is too long..
> >          //        currently the only user is FileXfer::handle_start
> > -        //        which verifies that strlen(_name) < MAX_PATH
> > -        vdagent_strcpy_s(name, sizeof(name), _name);
> > +        //        which verifies that _tcslen(_name) < MAX_PATH
> > +        lstrcpyn(name, _name, ARRAYSIZE(name));
> > +        name[ARRAYSIZE(name)-1] = 0;
> 
> We switched from using strncpy to using strcpy_s in commit d752a5b6e0 to
> fix warnings in Visual Studio. Any idea if they are going to come back
> with lstrcpyn?
> 
> Christophe
> 

Should not, at least not in Visual Studio 2008.

Frediano