[Spice-devel] [spice-common v4 7/7] log: Introduce spice_assert_if_fail

[Frediano Ziglio]

> 
> On Tue, Jan 26, 2016 at 06:53:20AM -0500, Frediano Ziglio wrote:
> > > 
> > > They can be used in spice-server to replace spice_return_if_fail.
> > > Currently spice_return_if_fail aborts in spice-server, and it's not
> > > always clear whether using a non-aborting g_return_if_fail is acceptable
> > > or not. Having a spice_assert_if_fail alternative makes it clearer that
> > > this is not going to return, while having a name distinct from assert()
> > > so that places which needs reviewing can be spotted more easily.
> > > 
> > > Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> > > ---
> > >  common/log.h         |  3 +++
> > >  tests/test-logging.c | 12 ++++++++++++
> > >  2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/common/log.h b/common/log.h
> > > index 0e03f59..b83b0e0 100644
> > > --- a/common/log.h
> > > +++ b/common/log.h
> > > @@ -95,6 +95,9 @@ void spice_log(const char *log_domain,
> > >      }                                                   \
> > >  } G_STMT_END
> > >  
> > > +#define spice_assert_val_if_fail(cond, val) spice_assert(cond)
> > 
> > Honestly this macro does not make much sense to me.
> > spice_assert is supposed to never return so why having a macro to
> > accept a return value and not returning?
> 
> Currently spice_return_if_fail() spice_return_val_if_fail() always
> assert() despite their name. The goal of this macro is that we do a mass
> renaming from spice_return_* to spice_assert_*. Then over time they can
> be changed to the appropriate g_assert() or g_return_* depending on what
> was actually meant. I'm fine with keeping this patch out for now though.
> 
> Christophe
> 

Why instead you merge the other patches and split this one removing that
macro definition?

Frediano