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

[Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Cody Chan 
On windows guest, after dragging 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.

​
​
---
 gtk/channel-main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index 7a299a4..1e8e3f5 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -936,7 +936,7 @@ static void
​​
agent_msg_queue_many(SpiceMainChannel *channel, int type, const void
 memcpy(payload, msg, sizeof(VDAgentMessage));
 payload += sizeof(VDAgentMessage);
 paysize -= sizeof(VDAgentMessage);
-if (paysize == 0) {
+if (size != 0  paysize == 0) {
 g_queue_push_tail(c-agent_msg_queue, out);
 out = NULL;
 }
@@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject *source_object,
 return;
 }

-if (count  0) {
+if (count = 0) {
 task-read_bytes += count;
 file_xfer_queue(task, count);
 file_xfer_flush_async(channel, task-cancellable,
--
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] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Marc-André Lureau 


- 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.

That's an an incompatible change. I would prefer if we keep sending the 
xfer-data message, even if it is of size 0.

Or if you prefer don't initiate xfer at all if the file size is 0. But I don't 
think that's worth it, and it would be some racy code (check size before 
doing actual operation)

 ​
 ​
 ---
 gtk/channel-main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/gtk/channel-main.c b/gtk/channel-main.c
 index 7a299a4..1e8e3f5 100644
 --- a/gtk/channel-main.c
 +++ b/gtk/channel-main.c
 @@ -936,7 +936,7 @@ static void
 ​​
 agent_msg_queue_many(SpiceMainChannel *channel, int type, const void
 memcpy(payload, msg, sizeof(VDAgentMessage));
 payload += sizeof(VDAgentMessage);
 paysize -= sizeof(VDAgentMessage);
 - if (paysize == 0) {
 + if (size != 0  paysize == 0) {
 g_queue_push_tail(c-agent_msg_queue, out);
 out = NULL;
 }
 @@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject *source_object,
 return;
 }
 
 - if (count  0) {
 + if (count = 0) {
 task-read_bytes += count;
 file_xfer_queue(task, count);
 file_xfer_flush_async(channel, task-cancellable,
 --
 1.9.3 ​
 
 --
 Q S B D T 0 R F U i B G U k 9 N I F J J R V N U I E 9 G I E N U U 0V V
 
 ___
 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

Re: [Spice-devel] [PATCH] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Cody Chan 
On Tue, Aug 12, 2014 at 12:58 AM, Marc-André Lureau mlur...@redhat.com
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.

 That's an an incompatible change. I would prefer if we keep sending the
 xfer-data message, even if it is of size 0.

 Or if you prefer don't initiate xfer at all if the file size is 0. But I
 don't think that's worth it, and it would be some racy code (check size
 before doing actual operation)

​You mean client shouldn't send a zero-size file?I think sending such file
is ​meaningless, too.
And I prefer this can be fixed through a more comfortable way.

  ​
  ​
  ---
  gtk/channel-main.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
  diff --git a/gtk/channel-main.c b/gtk/channel-main.c
  index 7a299a4..1e8e3f5 100644
  --- a/gtk/channel-main.c
  +++ b/gtk/channel-main.c
  @@ -936,7 +936,7 @@ static void
  ​​
  agent_msg_queue_many(SpiceMainChannel *channel, int type, const void
  memcpy(payload, msg, sizeof(VDAgentMessage));
  payload += sizeof(VDAgentMessage);
  paysize -= sizeof(VDAgentMessage);
  - if (paysize == 0) {
  + if (size != 0  paysize == 0) {
  g_queue_push_tail(c-agent_msg_queue, out);
  out = NULL;
  }
  @@ -1637,7 +1637,7 @@ static void file_xfer_read_cb(GObject
 *source_object,
  return;
  }
 
  - if (count  0) {
  + if (count = 0) {
  task-read_bytes += count;
  file_xfer_queue(task, count);
  file_xfer_flush_async(channel, task-cancellable,
  --
  1.9.3 ​
 
  --
  Q S B D T 0 R F U i B G U k 9 N I F J J R V N U I E 9 G I E N U U 0V V
 
  ___
  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] [spice-gtk] DND: fix special case that file size is 0

2014-08-11 Thread Marc-André Lureau 


- Original Message -
 
 
 
 On Tue, Aug 12, 2014 at 12:58 AM, Marc-André Lureau  mlur...@redhat.com 
 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.
 
 That's an an incompatible change. I would prefer if we keep sending the
 xfer-data message, even if it is of size 0.
 
 Or if you prefer don't initiate xfer at all if the file size is 0. But I
 don't think that's worth it, and it would be some racy code (check size
 before doing actual operation)
 ​You mean client shouldn't send a zero-size file?I think sending such file is
 ​meaningless, too.
 And I prefer this can be fixed through a more comfortable way.

I was talking from a protocol pov. But actually, sending 0-size file is 
meaningful in some cases. So it should keep sending it.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel