Re: [Spice-devel] spice related qemu crash
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
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
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
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
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 '['
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 '['
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 '['
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
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
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