[Spice-devel] [PATCH 08/24] server/red_channel: add red_channel_pipe_add_after (from red_worker)

[Alon Levy]

On Wed, Feb 02, 2011 at 07:51:34PM +0100, Marc-André Lureau wrote:
> On Wed, Jan 19, 2011 at 7:07 PM, Alon Levy <alevy at redhat.com> wrote:
> > ---
> >  server/red_channel.c |    8 ++++++++
> >  server/red_channel.h |    1 +
> >  2 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/server/red_channel.c b/server/red_channel.c
> > index 9b25f0a..68dfe9f 100644
> > --- a/server/red_channel.c
> > +++ b/server/red_channel.c
> > @@ -520,6 +520,14 @@ void red_channel_pipe_add_push(RedChannel *channel, PipeItem *item)
> >     red_channel_push(channel);
> >  }
> >
> > +void red_channel_pipe_add_after(RedChannel *channel, PipeItem *item, PipeItem *pos)
> > +{
> > +    ASSERT(channel && pos);
> 
> Can we break this assert in two lines, so it's easier to spot the problem.
> 
> Should it check for item != NULL as well? Since we are dereferencing it later.
Yes, I'll fix.

> 
> Here as well, I would prefer a return_if_fail() instead of assert().
> 
I think maybe a function called "program_error()" and a systemtap script to catch those
would do probably.

Specifically what you are saying is that we can drop random messages we are trying
to send, and try to continue. It would probably work - I mean the guest is happy, so
it won't crash, just the display would be wierd, and trying to find out why would be
pretty hard.

> > +    channel->pipe_size++;
> > +    ring_add_after(&item->link, &pos->link);
> > +}
> > +
> >  int red_channel_pipe_item_is_linked(RedChannel *channel, PipeItem *item)
> >  {
> >     return ring_item_is_linked(&item->link);
> > diff --git a/server/red_channel.h b/server/red_channel.h
> > index 08550a1..2bb611f 100644
> > --- a/server/red_channel.h
> > +++ b/server/red_channel.h
> > @@ -216,6 +216,7 @@ void red_channel_begin_send_message(RedChannel *channel);
> >  void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type);
> >  void red_channel_pipe_add_push(RedChannel *channel, PipeItem *item);
> >  void red_channel_pipe_add(RedChannel *channel, PipeItem *item);
> > +void red_channel_pipe_add_after(RedChannel *channel, PipeItem *item, PipeItem *pos);
> >  int red_channel_pipe_item_is_linked(RedChannel *channel, PipeItem *item);
> >  void red_channel_pipe_item_remove(RedChannel *channel, PipeItem *item);
> >  void red_channel_pipe_add_tail(RedChannel *channel, PipeItem *item);
> > --
> > 1.7.3.4
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> 
> 
> 
> -- 
> Marc-André Lureau