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

2014-08-13 Thread Christophe Fergeau 
On Tue, Aug 12, 2014 at 06:52:00PM +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 | 16 +++-
  1 file changed, 15 insertions(+), 1 deletion(-)
 
 diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
 index e402eb2..96b7394 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,20 @@ 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){
 +vd_printf(failed getting WideChar length of %s, file_path);
 +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);
 +if (wfile_path)
 +delete[] wfile_path;

delete[] NULL; is a noop so the explicit check for wfile__path is not
needed. I'll fix this before pushing this patch. ACK :)

Thanks!

Christophe


pgpYV8sohYi8M.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-13 Thread Christophe Fergeau 
Hi Cody,

On Tue, Aug 12, 2014 at 11:40:58AM +0200, Christophe Fergeau wrote:
 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

I've tried to apply your patches to the upstream git repository, but
your mailer seems to have corrupted them. Could you try to send them
using git send-email, or if this is not working for you, could you use
git format-patch and send them as an attachment? This would be very
helpful, thanks!

Christophe


pgpS1usMrMZob.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-13 Thread Cody Chan 
The attach is the patch​.


On Wed, Aug 13, 2014 at 10:47 PM, Cody Chan int64...@gmail.com wrote:

 But you ACKed this:
 http://lists.freedesktop.org/archives/spice-devel/2014-August/017209.html
 and this's my final version, doesn't it work?


 On Wed, Aug 13, 2014 at 10:27 PM, Christophe Fergeau cferg...@redhat.com
 wrote:

 Hi Cody,

 On Tue, Aug 12, 2014 at 11:40:58AM +0200, Christophe Fergeau wrote:
  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

 I've tried to apply your patches to the upstream git repository, but
 your mailer seems to have corrupted them. Could you try to send them
 using git send-email, or if this is not working for you, could you use
 git format-patch and send them as an attachment? This would be very
 helpful, thanks!

 Christophe




 --
 QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV




-- 
QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV


0001-fix-bug-display-error-when-dragging-file-with-CJK-ch.patch
Description: Binary data
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-13 Thread Christophe Fergeau 
On Wed, Aug 13, 2014 at 10:47:14PM +0800, Cody Chan wrote:
 But you ACKed this:
 http://lists.freedesktop.org/archives/spice-devel/2014-August/017209.html
 and this's my final version, doesn't it work?

Since you said you tested this patch and it works for you, I'm trusting
your tests, I haven't tried that code yet, but I feel comfortable enough
to ACK it.

Christophe

 
 
 On Wed, Aug 13, 2014 at 10:27 PM, Christophe Fergeau cferg...@redhat.com
 wrote:
 
  Hi Cody,
 
  On Tue, Aug 12, 2014 at 11:40:58AM +0200, Christophe Fergeau wrote:
   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
 
  I've tried to apply your patches to the upstream git repository, but
  your mailer seems to have corrupted them. Could you try to send them
  using git send-email, or if this is not working for you, could you use
  git format-patch and send them as an attachment? This would be very
  helpful, thanks!
 
  Christophe
 
 
 
 
 -- 
 QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV

 ___
 Spice-devel mailing list
 Spice-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/spice-devel



pgp8ZpgpfDfNo.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-13 Thread Cody Chan 
On Wed, Aug 13, 2014 at 11:13 PM, Christophe Fergeau cferg...@redhat.com
wrote:

 On Wed, Aug 13, 2014 at 10:47:14PM +0800, Cody Chan wrote:
  But you ACKed this:
 
 http://lists.freedesktop.org/archives/spice-devel/2014-August/017209.html
  and this's my final version,
 ​​
 doesn't it work?

 Since you said you tested this patch and it works for you, I'm trusting
 your tests, I haven't tried that code yet, but I feel comfortable enough
 to ACK it.

Christophe

​Of course, I test it ​carefully, 
​
doesn't it work? I mean if it's ok to patch to upstream.
Any way, I gave the patch as attachment just now, and I'm so sorry for
my unskilled
work.
Thx you again, you've really encouraged me.

  
 
  On Wed, Aug 13, 2014 at 10:27 PM, Christophe Fergeau 
 cferg...@redhat.com
  wrote:
 
   Hi Cody,
  
   On Tue, Aug 12, 2014 at 11:40:58AM +0200, Christophe Fergeau wrote:
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
  
   I've tried to apply your patches to the upstream git repository, but
   your mailer seems to have corrupted them. Could you try to send them
   using git send-email, or if this is not working for you, could you use
   git format-patch and send them as an attachment? This would be very
   helpful, thanks!
  
   Christophe
  
 
 
 
  --
  QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV

  ___
  Spice-devel mailing list
  Spice-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/spice-devel




-- 
QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-13 Thread Christophe Fergeau 
On Wed, Aug 13, 2014 at 11:21:11PM +0800, Cody Chan wrote:
 On Wed, Aug 13, 2014 at 11:13 PM, Christophe Fergeau cferg...@redhat.com
 wrote:
 
  On Wed, Aug 13, 2014 at 10:47:14PM +0800, Cody Chan wrote:
   But you ACKed this:
  
  http://lists.freedesktop.org/archives/spice-devel/2014-August/017209.html
   and this's my final version,
  ​​
  doesn't it work?
 
  Since you said you tested this patch and it works for you, I'm trusting
  your tests, I haven't tried that code yet, but I feel comfortable enough
  to ACK it.
 
 Christophe
 
 ​Of course, I test it ​carefully, 
 ​
 doesn't it work? I mean if it's ok to patch to upstream.

Oh I'm sorry I misunderstood your previous email, I indeed have replied
to the wrong patch, I did not understand this is why you said I ACK'ed
this other patch :) It's all good now, and I'm confident your patch
works :)

 Any way, I gave the patch as attachment just now, and I'm so sorry for
 my unskilled
 work.
 Thx you again, you've really encouraged me.

Well, thanks for diagnosing this bug in the first place, and for sending
and testing several iterations of this patch very quickly!

Christophe


pgpZgqiX3ZE42.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-12 Thread 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


pgp3zAFCiO3kt.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

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

2014-08-11 Thread Cody Chan 
Sorry, memory leaks..


On Mon, Aug 11, 2014 at 7:43 PM, Cody Chan int64...@gmail.com 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 | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

 diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
 index e402eb2..6e70d63 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,14 @@ 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;
 +TCHAR *wfile_path = (TCHAR*)malloc(sizeof(TCHAR) * (wlen + 1));
 +if (MultiByteToWideChar(CP_UTF8, 0, file_path, -1, wfile_path, wlen)
 == 0)
 +return;
 +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;
 --
 1.9.3

 --
 QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV




-- 
QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel