Re: [Spice-devel] spice related qemu crash

2014-08-12 Thread David Mansfield


On 08/12/2014 05:10 AM, Christophe Fergeau wrote:

On Mon, Aug 11, 2014 at 03:34:07PM -0400, David Mansfield wrote:

Hi All,

I have a qemu 1.6.2 (SRPM from F20 recompiled on Centos 7) which crashed
with the following in the qemu log.

Not sure exactly what the timing of the messages are here, since the
messages are not all timestamped:

Is this reproducible by any chance? I assume not?


Not so far. I'll let you know if I get any more info - just thought it 
might ring a bell.


Thanks,
David

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


Re: [Spice-devel] [spice-gtk] Ensure '\0' is not part of text clipboard data

2014-08-12 Thread Marc-André Lureau
ack

- Original Message -
> On Windows, with some versions of gtk+, GtkSelectionData::length
> will include the final '\0'.
> When a string with this trailing '\0' is pasted in some linux
> applications, it will be pasted as  or as an invisible character,
> which is unwanted.
> 
> This commit ensures the length we send to the agent does not
> include any trailing '\0'.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1090122
> ---
>  gtk/spice-gtk-session.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
> index db5c53c..84a83a2 100644
> --- a/gtk/spice-gtk-session.c
> +++ b/gtk/spice-gtk-session.c
> @@ -890,6 +890,15 @@ static void clipboard_received_cb(GtkClipboard
> *clipboard,
>  }
>  
>  len = strlen(conv);
> +} else {
> +/* On Windows, with some versions of gtk+, GtkSelectionData::length
> + * will include the final '\0'. When a string with this trailing
> '\0'
> + * is pasted in some linux applications, it will be pasted as 
> or
> + * as an invisible character, which is unwanted. Ensure the length
> we
> + * send to the agent does not include any trailing '\0'
> + * This is gtk+ bug
> https://bugzilla.gnome.org/show_bug.cgi?id=734670
> + */
> +len = strlen((const char *)data);
>  }
>  
>  spice_main_clipboard_selection_notify(s->main, selection, type,
> --
> 1.9.3
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [spice-gtk] Ensure '\0' is not part of text clipboard data

2014-08-12 Thread Christophe Fergeau
On Windows, with some versions of gtk+, GtkSelectionData::length
will include the final '\0'.
When a string with this trailing '\0' is pasted in some linux
applications, it will be pasted as  or as an invisible character,
which is unwanted.

This commit ensures the length we send to the agent does not
include any trailing '\0'.

https://bugzilla.redhat.com/show_bug.cgi?id=1090122
---
 gtk/spice-gtk-session.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
index db5c53c..84a83a2 100644
--- a/gtk/spice-gtk-session.c
+++ b/gtk/spice-gtk-session.c
@@ -890,6 +890,15 @@ static void clipboard_received_cb(GtkClipboard *clipboard,
 }
 
 len = strlen(conv);
+} else {
+/* On Windows, with some versions of gtk+, GtkSelectionData::length
+ * will include the final '\0'. When a string with this trailing '\0'
+ * is pasted in some linux applications, it will be pasted as  or
+ * as an invisible character, which is unwanted. Ensure the length we
+ * send to the agent does not include any trailing '\0'
+ * This is gtk+ bug https://bugzilla.gnome.org/show_bug.cgi?id=734670
+ */
+len = strlen((const char *)data);
 }
 
 spice_main_clipboard_selection_notify(s->main, selection, type,
-- 
1.9.3

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


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

2014-08-12 Thread Cody Chan
​
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;
+return;
+}
+handle = CreateFile(wfile_path, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0,
NULL);
+if (wfile_path)
+delete[] wfile_path;
 if (handle == INVALID_HANDLE_VALUE) {
 vd_printf("failed creating %s %lu", file_path, GetLastError());
 return;
--
1.9.3

-- 
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-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: g_key_get_string() of vd_agent failed when keystring contains '['

2014-08-12 Thread Christophe Fergeau
On Tue, Aug 12, 2014 at 11:20:21AM +0200, Christophe Fergeau wrote:
> On Mon, Aug 11, 2014 at 06:38:17PM +0800, Cody Chan wrote:
> > Yeah!I think yours is elegant and simple!
> 
> Have you tested this patch and verified it works? 

Ah sorry, I just saw your new version of the patch with this code where
you say you tested it :) Thanks!

Christophe


pgpyeqjVXtYN3.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: g_key_get_string() is failed when keystring contains '['

2014-08-12 Thread Christophe Fergeau
ACK

On Mon, Aug 11, 2014 at 08:21:49PM +0800, Cody Chan wrote:
> In vd_agent/file_xfer.cpp, it implemented a simple g_key_get_string,
> but  when dragging a file with a name containing '[' (like te[st.txt), it
> will be failed.
> From source code,
> >next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
> >if (next_group_pos && key_pos > next_group_pos) return false;
> we know that it tries to find the end of current group by '[' label,
> if we drag a file like te[st.txt, the key_string many like:
> [vdagent-file-xfer]
> name=te[st.txt
> size=10
> so, it's failed when meta parsing and
> returns VD_AGENT_FILE_XFER_STATUS_ERRO message.
> 
> Here's the elegant method Christophe proposed and test ok, thanks for him
> again!
> 
> ​
> ​
> ---
>  vdagent/file_xfer.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> index 1108369..cec5579 100644
> --- a/vdagent/file_xfer.cpp
> +++ b/vdagent/file_xfer.cpp
> @@ -205,7 +205,7 @@ bool FileXfer::g_key_get_string(char* data, const char*
> group, const char* key,
>  snprintf(key_pfx, sizeof(key_pfx), "\n%s=", key);
>  if (!(key_pos = strstr(group_pos, key_pfx))) return false;
> 
> -next_group_pos = strstr(group_pos + strlen(group_pfx), "[");
> +next_group_pos = strstr(group_pos + strlen(group_pfx), "\n[");
>  if (next_group_pos && key_pos > next_group_pos) return false;
> 
>  start = key_pos + strlen(key_pfx);
> --
> 1.9.3​
> 
> -- 
> QSBDT0RFUiBGUk9NIFJJRVNUIE9GIENUU0VV

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



pgpI2bH6v3G5a.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: g_key_get_string() of vd_agent failed when keystring contains '['

2014-08-12 Thread Christophe Fergeau
On Mon, Aug 11, 2014 at 06:38:17PM +0800, Cody Chan wrote:
> Yeah!I think yours is elegant and simple!

Have you tested this patch and verified it works? I did not try it at
all, but it seems to make sense when reading the code and your
explanations.

Christophe


pgpcTQRFSuMiU.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] [spice-gtk] DND: fix special case that file size is 0

2014-08-12 Thread Christophe Fergeau
Hey,

On Mon, Aug 11, 2014 at 12:58:44PM -0400, Marc-André Lureau wrote:
> 
> 
> - Original Message -
> > On windows guest, after dra gging a zero-size file, the file will be 
> > occupied
> > by vdagent. For spice-gtk ignores this case, and stops sending data after
> > sending
> > a start message to agent, see here for more details:
> > http://lists.freedesktop.org/archives/spice-devel/2014-August/017184.html
> > I accept the suggestion of Christophe and fix the bug in spice-gtk.
> > Pay attention to the modification of
> > ​
> > agent_msg_queue_many, I think it's a small trick.
> 
> Isn't this changing the protocol?
> 
> We use to send xfer-start of data size 0 and then xfer-data of data size 0, 
> and now we skip the second part.
> 

Actually, I think this is what causes this windows agent bug, with a
file whose size is 0, we never send a xfer-data message containing 0
bytes while the agent is waiting for that.
Something like this might do the trick... (totally untested)

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index 7a299a4..6eea260 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject
*source_object,
 return;
 }
 
-if (count > 0) {
+if ((count > 0) || (task->file_size == 0)) {
 task->read_bytes += count;
 file_xfer_queue(task, count);
 file_xfer_flush_async(channel, task->cancellable,

Christophe


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


Re: [Spice-devel] spice related qemu crash

2014-08-12 Thread Christophe Fergeau
On Mon, Aug 11, 2014 at 03:34:07PM -0400, David Mansfield wrote:
> Hi All,
> 
> I have a qemu 1.6.2 (SRPM from F20 recompiled on Centos 7) which crashed
> with the following in the qemu log.
> 
> Not sure exactly what the timing of the messages are here, since the
> messages are not all timestamped:

Is this reproducible by any chance? I assume not?

Christophe


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