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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> 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.

> > > -void spice_logv(const char *log_domain,
> > > -                SpiceLogLevel log_level,
> > > -                const char *strloc,
> > > -                const char *function,
> > > -                const char *format,
> > > -                va_list args) SPICE_ATTR_PRINTF(5, 0);
> > > -
> > 
> > Sure does not break anything?
> 
> Not that I know of, I'll try rebuilding spice-gtk as I mainly tested
> with spice-server.
> 

Using grep seems is not used.

> Christophe
> 

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]