[Spice-devel] [PATCH 11/18] worker: use spice_return_if_fail() instead of spice_assert() in release_item

[Christophe Fergeau]

On Tue, Nov 24, 2015 at 05:17:57AM -0500, Frediano Ziglio wrote:
> > 
> > On Mon, 2015-11-23 at 17:01 +0000, Frediano Ziglio wrote:
> > > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > > 
> > > ---
> > >  server/red_worker.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/server/red_worker.c b/server/red_worker.c
> > > index 656f9ab..65d5dea 100644
> > > --- a/server/red_worker.c
> > > +++ b/server/red_worker.c
> > > @@ -4453,7 +4453,7 @@ static void release_item(RedChannelClient *rcc,
> > > PipeItem
> > > *item, int item_pushed)
> > >  {
> > >      DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> > >  
> > > -    spice_assert(item);
> > > +    spice_return_if_fail(item != NULL);
> > >      dcc_release_item(dcc, item, item_pushed);
> > >  }
> > >  
> > 
> > Early return seems fine here, but in order to actually return and not abort,
> > we
> > should use g_return_if_fail()
> > 
> 
> I like spice_assert here.
> However changing to spice_return_if_fail would be a no-op and is not
> useful to mark a future conversion to g_return_if_fail.
> 
> I could have an easy suggestion if we decide we want to change the
> current spice_assert with a future g_return_if_fail:
> 
> before:
>   spice_assert(whatever);
> 
> after:
>   /* change to g_return_if_fail */
>   spice_return_if_fail(whatever);
> 
> This is the second proposal I do... I'm starting to be tired of
> these chit-chat.

If g_return_if_fail() is deemed good-enough here, why not use it now and
have one less spice_return_if_fail() to come back to and change?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151124/b8e9b439/attachment.sig>