After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 775879 - g_log_default_handler should not check G_MESSAGES_DEBUG
g_log_default_handler should not check G_MESSAGES_DEBUG
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
2.50.x
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-12-09 12:38 UTC by Ignacio Casal Quinteiro (nacho)
Modified: 2017-03-24 15:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmessages: Don’t check G_MESSAGES_DEBUG in old logging API (1.52 KB, patch)
2017-03-24 10:52 UTC, Philip Withnall
committed Details | Review

Description Ignacio Casal Quinteiro (nacho) 2016-12-09 12:38:59 UTC
Now that g_log_default_handler ends up calling the structured log handler the g_log one should not check for G_MESSAGES_DEBUG.

The reason for this is when you define G_LOG_USE_STRUCTURED and you only override the structured log handler. At this point you want to still get in your overridden handler the log messages that are coming from g_log, in fact the structured log handler already checks for G_MESSAGES_DEBUG.
Comment 1 Philip Withnall 2016-12-12 15:21:01 UTC
Seems reasonable. Patches welcome!
Comment 2 Philip Withnall 2017-03-24 10:52:00 UTC
Created attachment 348638 [details] [review]
gmessages: Don’t check G_MESSAGES_DEBUG in old logging API

Now that the old logging API forwards through to the new structured
logging API, we don’t need to check the level or domain of a message
against INFO_LEVELS or G_MESSAGES_DEBUG — just pass it straight through
to the new structured logging API writer function.
Comment 3 Philip Withnall 2017-03-24 10:52:53 UTC
Something like this then? I haven’t really tested it.
Comment 4 Ignacio Casal Quinteiro (nacho) 2017-03-24 11:09:50 UTC
Review of attachment 348638 [details] [review]:

Looks good.
Comment 5 Philip Withnall 2017-03-24 14:08:23 UTC
(In reply to Ignacio Casal Quinteiro (nacho) from comment #4)
> Review of attachment 348638 [details] [review] [review]:
> 
> Looks good.

Have you tested it with your project which was having problems? I haven’t got a relevant test case on hand for it, so would appreciate some real-world feedback before pushing.
Comment 6 Ignacio Casal Quinteiro (nacho) 2017-03-24 14:35:44 UTC
Nope but on my project I basically did something like this:

static void
log_handler(const gchar    *log_domain,
            GLogLevelFlags  log_level,
            const gchar    *message,
            gpointer        useless)
{
    GLogField fields[2];
    int n_fields = 0;

    fields[n_fields].key = "MESSAGE";
    fields[n_fields].value = message;
    fields[n_fields].length = -1;
    n_fields++;

    if (log_domain) {
        fields[n_fields].key = "GLIB_DOMAIN";
        fields[n_fields].value = log_domain;
        fields[n_fields].length = -1;
        n_fields++;
    }

    g_log_structured_array(log_level & ~G_LOG_FLAG_FATAL, fields, n_fields);
}
Comment 7 Philip Withnall 2017-03-24 15:07:54 UTC
Shipping it.
Comment 8 Philip Withnall 2017-03-24 15:08:51 UTC
Attachment 348638 [details] pushed as 21b6b1f - gmessages: Don’t check G_MESSAGES_DEBUG in old logging API