[Spice-devel] SPICE logging facilities

[Christophe Fergeau]

On Wed, Jul 04, 2018 at 12:29:15PM -0400, Frediano Ziglio wrote:
> > 
> > On Wed, Jul 04, 2018 at 11:38:14AM -0400, Frediano Ziglio wrote:
> > > Another question is however "Are we going to use g_critical as
> > > g_critical?". It sounds a tricky question. Let say that a new person
> > > starts to look at the code and knows GLib. He see g_critical and
> > > think "well, this by default log a critical warning and continue"
> > > but instead on Spice is always fatal.
> > 
> > Unless I am confused, g_critical() should have the usual default
> > behaviour, and spice_critical() aborts, see
> > test_spice_abort_level_g_warning and the following tests
> > https://gitlab.freedesktop.org/spice/spice-common/blob/master/tests/test-logging.c#L62
> > 
> > Christophe
> > 
> 
> But you suggested c3d to use g_critical instead of spice_critical,
> isn't it confusing?

In the context of c3d's patch, the added spice_critical calls should not
be fatal, so g_critical is fine.

> 
> Forgot about telling what I think about logging and g_XXX vs spice_XXX.
> I agree we should use a single API to avoid confusion but should be
> consistent, not introducing free regressions so spice_critical -> g_error
> and spice_return_if_fail/spice_return_val_if_fail/spice_assert -> g_assert
> (making sure it's never disabled!).

If we replace preexisting logging calls and err on the very safe side,
yes I agree. But I'd prefer that we don't switch everything with sed,
and audit each call instead, I suspect most of the time not asserting
will be ok.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180705/533c5895/attachment.sig>