On Tue, Feb 08, 2011 at 07:47:23PM +0100, Marc-André Lureau wrote:
> Hi Alon, Gerd,
>
> While reviewing Alon patches, I started to get annoyed by a crash I
> didn't see before, although it seems to be an old bug, and comes from
> qemu perhaps?
>
> I am investing that crash before continuing the r
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy wrote:
> Split it out of display_channel_push.
> ---
> server/red_worker.c | 182
> ++-
> 1 files changed, 94 insertions(+), 88 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy wrote:
> s/red_pipe_add/red_channel_pipe_add/
> s/red_pipe_add_after/red_channel_pipe_add_after/
> ---
> server/red_worker.c | 32
> 1 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/server/red_wo
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy wrote:
> ---
> server/red_worker.c | 34 +-
> 1 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 0ed46e9..6bd8bc2 100644
> --- a/server/red_worker.c
Please merge with patch 03 (keep the comment though)
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy wrote:
> Added cb takes care of non zero offset embedded EventHandler, which happens
> now with the introduced CommonChannel.
> ---
> server/red_worker.c | 19 ++-
> 1 files changed,
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy wrote:
> start of move to red_channel based channels
> ---
> server/red_worker.c | 38 --
> 1 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index df
ack
Btw, I realize that in the previous patch series, we had
index e8ebb05..a778ffb 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -107,6 +107,7 @@ typedef void
(*channel_release_msg_recv_buf_proc)(RedChannel *channel,
typedef void (*channel_disconnect_proc)(RedChannel *channel)
Hopefully we can find a more elegant solution to the downcasting
introduced in patch 03: CommonChannel *common =
SPICE_CONTAINEROF(channel, CommonChannel, base);
I would rather modify the function common_release_pipe_item(RedChannel
*channel, PipeItem *item) to be:
common_release_pipe_item(CommonC
ack
On Mon, Feb 7, 2011 at 7:19 PM, Alon Levy wrote:
> Changes semantics of send to always hold/release regardless of block, like
> red_channel. A hold is just a reference count increment or nop.
> ---
> server/red_worker.c | 171
> +--
> 1 files
well... ack, but obviously while you are at it, you could rename the
rest as well?, they are plenty, for instance:
red_receive(RedChannel *channel); -> red_channel_receive
red_release_pixmap_cache(DisplayChannel *channel); ->
red_display_release_pixmap_cache
display_begin_send_message(DisplayChann
Looks like it should be squashed with patch 05.
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy wrote:
> ---
> server/red_worker.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index fb04ed9..e8849a7 100644
> --- a/server
Even if the function name is explicit, I would rather see:
cursor_channel_send_item(CursorChannel *cursor_channel, ...) {
RedChannel *channel = (CursorChannel *)cursor_channel;
}
to avoid downcasting, perhaps it's matter of taste, but I feel better
with upcasts.
On Mon, Feb 7, 2011 at 7:20
Ok, I would get rid of "out_" prefix in out_bytes_counter.
What's the motivation for this change?
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy wrote:
> ---
> server/red_worker.c | 10 ++
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_
hmm, again, it's obvious that there is a problem of naming and
consistency in the code. Just modifying one makes perhaps things
worst.. what about? :
hold_pipe_item_proc -> pipe_item_hold_proc
release_item_proc -> channel_release_item_proc
handle_message_proc -> channel_handle_message_proc
So, I
to merge with patch 13
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy wrote:
> ---
> server/red_worker.c | 7 ---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 0393d77..8c39d09 100644
> --- a/server/red_worker.c
> +++ b/se
(to merge with patch 13)
Perhaps channel_handle_parsed_proc instead?
Also, I suppose you should rename RedChannel.handle_message to
RedChannel.handle_parsed
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy wrote:
> Signature changed to red_channel's handle_parsed_proc, and rename to same,
> towards re
please merge with patch 13, it makes bisection faster (reviewing is
easy for this kind of renaming change, no need to split it).
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy wrote:
> ---
> server/red_worker.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/server/
Ack. I don't see the need for item_pushed argument, is it used later
in your patch series?
On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy wrote:
> ---
> server/red_worker.c | 19 ++-
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/server/red_worker.c b/server/re
On Tue, Feb 08, 2011 at 08:02:51PM +0200, Uri Lublin wrote:
> On 02/07/2011 03:11 PM, Alon Levy wrote:
> >---
> > server/red_worker.c |2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/server/red_worker.c b/server/red_worker.c
> >index f899ff2..f163a71 100644
> >--
Hi
On Tue, Feb 8, 2011 at 7:47 PM, Marc-André Lureau
wrote:
> Hi Alon, Gerd,
>
> While reviewing Alon patches, I started to get annoyed by a crash I
> didn't see before, although it seems to be an old bug, and comes from
> qemu perhaps?
>
> I am investing that crash before continuing the review,
Hi Alon, Gerd,
While reviewing Alon patches, I started to get annoyed by a crash I
didn't see before, although it seems to be an old bug, and comes from
qemu perhaps?
I am investing that crash before continuing the review, as it gets
worst when I applied the first 3/4 patches (double free..).
Th
On 02/07/2011 03:11 PM, Alon Levy wrote:
---
server/red_worker.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index f899ff2..f163a71 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -6219,7 +6219,7 @@ static in
22 matches
Mail list logo
