[Spice-devel] [PATCH] log: add not fatal spice_return function

[Frediano Ziglio]

> 
> Hey,
> 
> On Thu, Nov 19, 2015 at 06:58:41AM -0500, Frediano Ziglio wrote:
> > > 
> > > Hi
> > > 
> > > ----- Original Message -----
> > > > Due to implementation details spice_return is by default fatal (program
> > > > is aborting). This is quite confusing but changing the current macros
> > > > would possibly break existing code.
> > > > Add not fatal (at least by default) macros. They use warning level
> > > > instead of criticial.
> > > > This is also compatible with Glib macros.
> > > 
> > > Why not start using glib macros instead?
> > > 
> > 
> > starting would mean:
> > - have half call to old and half to new with 2 different behavior);
> > - switch all to glib changing behavior and making hard to switch back.
> > 
> > Adding new macros more compatible with glib but 100% compatible with
> > current
> > code make the transition more smooth.
> 
> What do you mean by "100% compatible with the current code"? (why is
> g_return_if-fail() not "100% compatible with the current code" ?)
> 

well... implementation is quite different. I didn't get all differences but
- spice_logv use some environment SPICE_* specific (I doubt Glib does!);
- Glib output on standard error or output based on level;
- surely something I forgot!

Didn't investigate so deep to be able to tell all list but surely just with these
I'm not comfortable to do a sed and release tomorrow...

Calling spice_* functions instead of spice_* functions looks like 100% compatible :)

Frediano

> > 
> > For instance we could change the assert to spice_return_*warning now
> > knowing
> > that we can safely convert to g_return* in the future.
> 
> I agree with Marc-André, it's better to directly glib macros here
> rather than introducing our own once again. We should try to have less
> duplication with glib, not more. The spice_return_if_fail situation is
> unfortunate (that it asserts), but I'd clarify this by (optionally)
> renaming spice_return_if_fail() and using g_return_if_fail() rather than
> having spice_return_if_fail() and spice_return_if_fail_warn().
> 
> Christophe
>