[Spice-devel] [PATCH 11/11] worker: remove assertion on alloc_drawable

[Frediano Ziglio]

> 
> On Fri, 2015-10-30 at 13:23 -0400, Frediano Ziglio wrote:
> > > 
> > > On Thu, Oct 29, 2015 at 04:24:27PM -0400, Frediano Ziglio wrote:
> > > > > 
> > > > > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > > > > 
> > > > > There is no guarantee in the code that this can't be hit, so we
> > > > > should
> > > > > cope with it (the condition can be reached easily by running
> > > > > the server
> > > > > without waiting for blocked clients or pipe size)
> > > > > 
> > > > > The following commit will attempt to address this.
> > > > > ---
> > > > >  server/red_worker.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/server/red_worker.c b/server/red_worker.c
> > > > > index 0542b29..ca7e8bf 100644
> > > > > --- a/server/red_worker.c
> > > > > +++ b/server/red_worker.c
> > > > > @@ -3300,7 +3300,8 @@ static void free_one_drawable(RedWorker
> > > > > *worker,
> > > > > int
> > > > > force_glz_free)
> > > > >      Drawable *drawable;
> > > > >      Container *container;
> > > > >  
> > > > > -    spice_assert(ring_item);
> > > > > +    if (!ring_item)
> > > > > +        return;
> > > > >      drawable = SPICE_CONTAINEROF(ring_item, Drawable,
> > > > > list_link);
> > > > >      if (force_glz_free) {
> > > > >          RingItem *glz_item, *next_item;
> > > > > --
> > > > > 2.4.3
> > > > > 
> > > > 
> > > > The change make sense. The function is called from different
> > > > places and not
> > > > in all path
> > > > we are sure worker->current_list is not empty.
> > > > 
> > > > Style note: looks like we prefer always bracket like
> > > > 
> > > > if (!ring_item) {
> > > >     return;
> > > > }
> > > > 
> > > > Beside the style note I would ack the patch.
> > > 
> > > Brackets would probably be more consistent yes, ack too.
> > > 
> > > Christophe
> > > 
> > > 
> > 
> > Pushed
> > 
> > Frediano
> > 
> 
> Hmm, you pushed already, but it seems that the commit summary doesn't
> match the change (alloc_drawable() vs free_one_drawable())
> 
> Oh well...
> 
> 

I think I should add this to my checklist before pushing

Frediano