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

[Victor Toso]

Hi,

On Tue, Jan 19, 2016 at 05:29:33PM +0100, Christophe Fergeau wrote:
> On Wed, Dec 16, 2015 at 03:15:35PM +0100, Victor Toso wrote:
> > Hi,
> > 
> > On Wed, Dec 16, 2015 at 02:27:13PM +0100, Christophe Fergeau 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)
> > 
> > I did not understand why you need spice_assert_val_if_fail if val is
> > ignored
>
> We currently have spice_return_if_fail/spice_return_val_if_fail which
> are similar to g_return_if_fail/g_return_val_if_fail, but abort. The
> naming is very confusing. The goal of these functions is to make it
> explicit that the function is aborting, and at the same time to mark the
> location as a good candidate for replacement by g_return_val_if_fail.
>
> Christophe

This is definetly not a blocker for this patch, so
Acked-by: Victor Toso <victortoso at redhat.com>

But I don't feel that one would be using spice_assert_val_if_fail
instead of the simpler version.

Cheers,
  toso