[Spice-devel] [spice-common v4 3/7] log: Use glib for logging

[Frediano Ziglio]

> 
> On Tue, Jan 26, 2016 at 06:49:55AM -0500, Frediano Ziglio wrote:
> > > 
> > > On Thu, Jan 21, 2016 at 07:29:59AM -0500, Frediano Ziglio wrote:
> > > > > +            /* Make sure GLib default log handler will show the
> > > > > debug
> > > > > messages. Messing with
> > > > > +             * environment variables like this is ugly, but this
> > > > > only
> > > > > happens when the legacy
> > > > > +             * SPICE_DEBUG_LEVEL is used
> > > > > +             */
> > > > > +            debug_env = (char *)g_getenv("G_MESSAGES_DEBUG");
> > > > > +            if (debug_env == NULL) {
> > > > > +                g_setenv("G_MESSAGES_DEBUG", SPICE_LOG_DOMAIN,
> > > > > FALSE);
> > > > > +            } else {
> > > > > +                debug_env = g_strconcat(debug_env, ":",
> > > > > SPICE_LOG_DOMAIN,
> > > > > NULL);
> > > > > +                g_setenv("G_MESSAGES_DEBUG", SPICE_LOG_DOMAIN,
> > > > > FALSE);
> > > > > +                g_free(debug_env);
> > > > 
> > > > A library should never set environment variable.
> > > 
> > > This is the only way I could find to keep SPICE_DEBUG_LEVEL keep working
> > > as it used to even when g_debug() is used. This code is being careful to
> > > only make somehow safe changes to that variable.
> > > This environment variable is only set in a legacy case (ie one I'd like
> > > to see go away), and this legacy case goes with a runtime warning
> > > encouraging the user to switch to setting G_MESSAGES_DEBUG instead (in
> > > which case we do not muck with env variables ourselves).
> > > In my opinion, it's reasonable enough, especially without an alternative
> > > to achieve that.
> > > 
> > 
> > Sounds reasonable. I'd add a TODO in the comment to remove in a couple of
> > versions.
> > I don't know if is possible to make code fail compiling if the version
> > bump a bit.
> 
> I've added /* FIXME: To be removed after enough deprecation time */
> Some #if magic on SPICE_VERSION_* and #error could achieve compilation
> failing after a while, but I think a comment is a good enough tradeoff.
> 
> Christophe
> 

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano