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@xxxxxxxxxx> > > > --- > > > 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@xxxxxxxxxx> But I don't feel that one would be using spice_assert_val_if_fail instead of the simpler version. Cheers, toso _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel