[Spice-devel] [PATCH 05/18] worker: change some asserts in stream functions

[Frediano Ziglio]

> 
> On Mon, 2015-11-23 at 17:01 +0000, Frediano Ziglio wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > 
> > ---
> >  server/stream.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/server/stream.c b/server/stream.c
> > index 8d16658..85136ea 100644
> > --- a/server/stream.c
> > +++ b/server/stream.c
> > @@ -263,8 +263,11 @@ static void attach_stream(DisplayChannel *display,
> > Drawable *drawable, Stream *s
> >      DisplayChannelClient *dcc;
> >      RingItem *item, *next;
> >  
> > -    spice_assert(!drawable->stream && !stream->current);
> > -    spice_assert(drawable && stream);
> > +    spice_return_if_fail(!drawable->stream);
> > +    spice_return_if_fail(!stream->current);
> > +    spice_return_if_fail(drawable);
> > +    spice_return_if_fail(stream);
> > +
> >      stream->current = drawable;
> >      drawable->stream = stream;
> >      stream->last_time = drawable->creation_time;
> > @@ -305,8 +308,10 @@ static void attach_stream(DisplayChannel *display,
> > Drawable *drawable, Stream *s
> >  void detach_stream(DisplayChannel *display, Stream *stream,
> >                     int detach_sized)
> >  {
> > -    spice_assert(stream->current && stream->current->stream);
> > -    spice_assert(stream->current->stream == stream);
> > +    spice_return_if_fail(stream->current);
> > +    spice_return_if_fail(stream->current->stream);
> > +    spice_return_if_fail(stream->current->stream == stream);
> > +
> >      stream->current->stream = NULL;
> >      if (detach_sized) {
> >          stream->current->sized_stream = NULL;
> > @@ -406,13 +411,13 @@ static void
> > display_channel_create_stream(DisplayChannel
> > *display, Drawable *dra
> >      Stream *stream;
> >      SpiceRect* src_rect;
> >  
> > -    spice_assert(!drawable->stream);
> > +    spice_return_if_fail(!drawable->stream);
> >  
> >      if (!(stream = display_channel_stream_try_new(display))) {
> >          return;
> >      }
> >  
> > -    spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
> > +    spice_return_if_fail(drawable->red_drawable->type == QXL_DRAW_COPY);
> >      src_rect = &drawable->red_drawable->u.copy.src_area;
> >  
> >      ring_add(&display->streams, &stream->link);
> 
> 
> These all seem safe for early returns instead of asserts, in which case I
> would
> advocate changing to g_return_if_fail()...
> 
> Jonathon
> 

I think that in all the above cases spice_assert is fine

NACK.

Frediano