[Spice-devel] [PATCH] [vd_agent] fix bug: display error when dragging file with CJK characters

[Christophe Fergeau]

Hey,

On Mon, Aug 11, 2014 at 07:58:44PM +0800, Cody Chan wrote:
> ​
> I submitted a patch several months ago about this issue,
> here
> http://lists.freedesktop.org/archives/spice-devel/2014-February/016158.html
> 
> I check it again, and find that the value of
> g_key_file_to_data(keyfile,...) is always utf-8,
> for the value of g_uri_list_extract_uris() is utf8 urlencode.
> 
> So the display problem is caused by vd_agent, but how it displays depends
> on the
> language of system, the following two screenshots show the difference:
> http://int64ago-tmp.qiniudn.com/guest-Chinese.png
> http://int64ago-tmp.qiniudn.com/guest-English.png
> 
> 
> ​
> ​
> ​
> ---
>  vdagent/file_xfer.cpp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index e402eb2..1108369 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -46,6 +46,7 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> start,
>      uint64_t file_size;
>      HANDLE handle;
>      AsUser as_user;
> +    int wlen;
> 
>      status->id = start->id;
>      status->result = VD_AGENT_FILE_XFER_STATUS_ERROR;
> @@ -81,7 +82,17 @@ void FileXfer::handle_start(VDAgentFileXferStartMessage*
> start,
> 
>      strcat(file_path, "\\");
>      strcat(file_path, file_name);
> -    handle = CreateFileA(file_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
> NULL);
> +    if((wlen = MultiByteToWideChar(CP_UTF8, 0, file_path, -1, NULL, 0)) ==
> 0)
> +        return;

You could log an error using vd_printf so that we know what's going on
if this fails.

> +    TCHAR *wfile_path = (TCHAR*)malloc(sizeof(TCHAR) * (wlen + 1));

Hmm, rest of the code is using 'new' which will raise an exception if we
run out of memory. If you use malloc, you need to check for NULL in case
of failure.
I don't think the '+1' is needed here as MultiByteToWideChar
documentation says:
"Size, in characters, of the buffer indicated by lpWideCharStr. If this
value is 0, the function returns the required buffer size, in
characters, including any terminating null character, and makes no use
of the lpWideCharStr buffer."

http://msdn.microsoft.com/en-us/library/windows/desktop/dd319072%28v=vs.85%29.aspx


> +    if (MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, wlen)
> == 0){
> +        if(wfile_path)
> +            free(wfile_path);
> +        return;

Same comment about adding a vd_printf log here.

> +    }
> +    handle = CreateFile(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
> NULL);
> +    if (wfile_path)
> +        free(wfile_path);
>      if (handle == INVALID_HANDLE_VALUE) {
>          vd_printf("failed creating %s %lu", file_path, GetLastError());
>          return;

Apart from these comments, patch looks good to me!

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140812/328a30a8/attachment-0001.sig>