On Thu, Dec 17, 2015 at 03:56:38PM -0600, Jonathon Jongsma wrote:
> > +
> > +static void spice_log_set_debug_level(void)
> > +{
> > + if (glib_debug_level == 0) {
> > + char *debug_str = getenv("SPICE_DEBUG_LEVEL");
> > + if (debug_str != NULL) {
> > + int debug_level;
> > + char *debug_env;
> > +
> > + g_warning("Setting SPICE_DEBUG_LEVEL is deprecated, use
> > G_MESSAGES_DEBUG instead");
> > + debug_level = atoi(getenv("SPICE_DEBUG_LEVEL"));
>
> Out of curiosity, why are you using getenv here again instead of just using the
> local debug_str variable you already set?
Oh, I believe this was mentioned before, forgot to change it, I'll do it
now.
>
>
> > + glib_debug_level = spice_log_level_to_glib(debug_level);
> > +
> > + /* If the debug level is too high, make sure we don't try to
> > enable
> > + * display of glib debug logs */
> > + if (debug_level < SPICE_LOG_LEVEL_INFO)
> > + return;
>
> Why? I don't understand this bit.
SPICE_DEBUG_LEVEL can be set to SPICE_LOG_LEVEL_DEBUG,
SPICE_LOG_LEVEL_INFO, in which case we need to set G_MESSAGES_DEBUG
to get a similar behaviour with g_log(). However, it can also be set
to SPICE_LOG_LEVEL_WARNING, SPICE_LOG_LEVEL_CRITICAL to hide messages
which would otherwise be shown by default. When this is the case,
setting G_MESSAGES_DEBUG is not useful (even though I don't think it
would be harmful as spice_logger() would filter out the messages
anyway).
>
> > +
> > + /* 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);
> > + }
> > + }
> > }
> > +}
> > +
> > +static void spice_log_set_abort_level(void)
> > +{
> > if (abort_level == -1) {
> > - abort_level = getenv("SPICE_ABORT_LEVEL") ?
> > atoi(getenv("SPICE_ABORT_LEVEL")) : SPICE_ABORT_LEVEL_DEFAULT;
> > + char *abort_str = getenv("SPICE_ABORT_LEVEL");
> > + if (abort_str != NULL) {
> > + GLogLevelFlags glib_abort_level;
> > + g_warning("Setting SPICE_ABORT_LEVEL is deprecated, use G_DEBUG
> > instead");
> > + abort_level = atoi(getenv("SPICE_ABORT_LEVEL"));
>
> again, why not use the local variable instead of re-calling getenv()?
>
> >
> > + glib_abort_level = spice_log_level_to_glib(abort_level);
> > + if (glib_abort_level != 0) {
> > + unsigned int fatal_mask = G_LOG_FATAL_MASK;
> > + while (glib_abort_level >= G_LOG_LEVEL_ERROR) {
> > + fatal_mask |= glib_abort_level;
> > + glib_abort_level >>= 1;
> > + }
> > + g_log_set_fatal_mask(SPICE_LOG_DOMAIN, fatal_mask);
> > + }
> > + } else {
> > + abort_level = SPICE_ABORT_LEVEL_DEFAULT;
> > + }
> > }
> > +}
> >
> > - if (debug_level < (int) log_level)
> > - return;
> > +static void spice_logger(const gchar *log_domain,
> > + GLogLevelFlags log_level,
> > + const gchar *message,
> > + gpointer user_data)
> > +{
> > + if (glib_debug_level != 0) {
> > + if ((log_level & G_LOG_LEVEL_MASK) > glib_debug_level)
> > + return; // do not print anything
> > + }
> > + g_log_default_handler(log_domain, log_level, message, NULL);
> > +}
> >
> > - fprintf(stderr, "(%s:%d): ", getenv("_"), getpid());
> > +static void spice_log_init_once(void)
>
> inline?
If you want, dunno how much it matters.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel