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