[virt-tools-list] [virt-viewer v2] log: Handle G_MESSAGES_DEBUG with older glibs

[Marc-André Lureau]

Sorry, I misread the patch.

1. for >2.31 add domain G_MESSAGES_DEBUG if --debug is passed
2. for <2.31 do not filter debug messages

I agree with 1., but I nack 2. since it was the initial goal of 2bd835fb.
However, it should only filter its own debug messages, and for that I
proposed another patch "*Only filter virt-viewer debug messages"
<https://www.redhat.com/archives/virt-tools-list/2014-July/msg00058.html>*


On Mon, Jul 14, 2014 at 8:08 PM, Marc-André Lureau <
marcandre.lureau at gmail.com> wrote:

> It's a nice idea, but I have to nack. Imho we shouldn't introduce
> G_MESSAGES_DEBUG handling for older glib that didn't support it.
>
>
> On Fri, Jun 27, 2014 at 6:39 PM, Christophe Fergeau <cfergeau at redhat.com>
> wrote:
>
>> Commit 2bd835fb introduced a custom log handler to ensure
>> we never show debug messages by default regardless of glib
>> version (older glib were showing them by default).
>>
>> However this custom log handler does not take into account
>> G_MESSAGES_DEBUG. This commit adds support for that and
>> makes the handling of --debug identical between older and newer glibs.
>> ---
>>
>> Changes since v1:
>> - on older glibs, only do the getenv dance once instead of once
>>   per log-message
>>
>>  src/virt-viewer-app.c  | 17 ++++++++++-------
>>  src/virt-viewer-util.c |  8 +++++---
>>  2 files changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
>> index 24ca5c1..a523090 100644
>> --- a/src/virt-viewer-app.c
>> +++ b/src/virt-viewer-app.c
>> @@ -183,19 +183,22 @@ static guint signals[SIGNAL_LAST];
>>  void
>>  virt_viewer_app_set_debug(gboolean debug)
>>  {
>> -#if GLIB_CHECK_VERSION(2, 31, 0)
>> -    if (debug) {
>> -        const gchar *doms = g_getenv("G_MESSAGES_DEBUG");
>> -        if (!doms) {
>> +    const gchar *doms = g_getenv("G_MESSAGES_DEBUG");
>> +    if (!doms) {
>> +        if (debug)
>>              g_setenv("G_MESSAGES_DEBUG", G_LOG_DOMAIN, 1);
>> -        } else if (!g_str_equal(doms, "all") &&
>> -                   !strstr(doms, G_LOG_DOMAIN)) {
>> +    } else if (!g_str_equal(doms, "all") &&
>> +               !strstr(doms, G_LOG_DOMAIN)) {
>> +        if (debug) {
>>              gchar *newdoms = g_strdup_printf("%s %s", doms,
>> G_LOG_DOMAIN);
>>              g_setenv("G_MESSAGES_DEBUG", newdoms, 1);
>>              g_free(newdoms);
>>          }
>> +    } else {
>> +        /* G_MESSAGES_DEBUG=all or G_MESSAGES_DEBUG contains
>> G_LOG_DOMAIN */
>> +        debug = TRUE;
>>      }
>> -#endif
>> +
>>      doDebug = debug;
>>  }
>>
>> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
>> index 6fdaa07..f4bd20a 100644
>> --- a/src/virt-viewer-util.c
>> +++ b/src/virt-viewer-util.c
>> @@ -283,9 +283,11 @@ static void log_handler(const gchar *log_domain,
>>                          const gchar *message,
>>                          gpointer unused_data)
>>  {
>> -    if (glib_check_version(2, 32, 0) != NULL)
>> -        if (log_level >= G_LOG_LEVEL_DEBUG && !doDebug)
>> -            return;
>> +#if !GLIB_CHECK_VERSION(2, 31, 0)
>> +    /* Older glibs were showing G_LOG_LEVEL_{INFO,DEBUG} messages by
>> default */
>> +    if (log_level >= G_LOG_LEVEL_INFO && !doDebug)
>> +        return;
>> +#endif
>>
>>      g_log_default_handler(log_domain, log_level, message, unused_data);
>>  }
>> --
>> 1.9.3
>>
>> _______________________________________________
>> virt-tools-list mailing list
>> virt-tools-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list
>>
>
>
>
> --
> Marc-André Lureau
>



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20140721/3257759d/attachment.htm>