From 310c3ed4cc733f47b88b44fb03757bd7213a4f9a Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Wed, 21 Sep 2011 21:20:07 -0400 Subject: [PATCH] Clean up process of calling g_debug_init() Make sure that it calls absolutely nothing that may ever recurse back into GLib again: - g_ascii_strcasecmp() is unsafe because it has g_return_if_fail() at the top. As far as I know, the only ASCII character letter that would get special treatment here is "i" and that appears in neither "help" nor "all". - g_getenv() is very complicated on Windows, so use a simple version that is sufficient for our purposes. Now that it's completely safe, we can just call it from g_logv() in the usual way without all of the hacks. https://bugzilla.gnome.org/show_bug.cgi?id=660744 --- glib/gmessages.c | 83 ++++++++++++++++++++++---------------------------------- glib/gutils.c | 16 +++++++---- 2 files changed, 43 insertions(+), 56 deletions(-) diff --git a/glib/gmessages.c b/glib/gmessages.c index c583188..1ed1e15 100644 --- a/glib/gmessages.c +++ b/glib/gmessages.c @@ -186,46 +186,44 @@ g_messages_prefixed_init (void) } } -static void -g_debug_init (void) +static guint +g_parse_debug_envvar (const gchar *envvar, + const GDebugKey *keys, + gint n_keys) { - typedef enum { - G_DEBUG_FATAL_WARNINGS = 1 << 0, - G_DEBUG_FATAL_CRITICALS = 1 << 1 - } GDebugFlag; - const gchar *val; - guint flags = 0; + const gchar *value; - g_debug_initialized = TRUE; +#ifdef OS_WIN32 + /* "fatal-warnings,fatal-criticals,all,help" is pretty short */ + gchar buffer[80]; - val = g_getenv ("G_DEBUG"); - if (val != NULL) - { - const GDebugKey keys[] = { - {"fatal_warnings", G_DEBUG_FATAL_WARNINGS}, - {"fatal_criticals", G_DEBUG_FATAL_CRITICALS} - }; + if (GetEnvironmentVariable (envvar, buffer, 100) < 100) + value = buffer; + else + return 0; +#else + value = getenv (envvar); +#endif - flags = g_parse_debug_string (val, keys, G_N_ELEMENTS (keys)); - } + return g_parse_debug_string (value, keys, n_keys); +} - if (flags & G_DEBUG_FATAL_WARNINGS) - { - GLogLevelFlags fatal_mask; +static void +g_debug_init (void) +{ + const GDebugKey keys[] = { + {"fatal-warnings", G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL }, + {"fatal-criticals", G_LOG_LEVEL_CRITICAL } + }; + GLogLevelFlags flags; - fatal_mask = g_log_set_always_fatal (G_LOG_FATAL_MASK); - fatal_mask |= G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL; - g_log_set_always_fatal (fatal_mask); - } + flags = g_parse_debug_envvar ("G_DEBUG", keys, G_N_ELEMENTS (keys)); - if (flags & G_DEBUG_FATAL_CRITICALS) - { - GLogLevelFlags fatal_mask; + g_mutex_lock (&g_messages_lock); + g_log_always_fatal |= flags; + g_mutex_unlock (&g_messages_lock); - fatal_mask = g_log_set_always_fatal (G_LOG_FATAL_MASK); - fatal_mask |= G_LOG_LEVEL_CRITICAL; - g_log_set_always_fatal (fatal_mask); - } + g_debug_initialized = TRUE; } static GLogDomain* @@ -500,6 +498,9 @@ g_logv (const gchar *log_domain, gboolean was_recursion = (log_level & G_LOG_FLAG_RECURSION) != 0; gint i; + if (!g_debug_initialized) + g_debug_init (); + log_level &= G_LOG_LEVEL_MASK; if (!log_level) return; @@ -542,24 +543,6 @@ g_logv (const gchar *log_domain, g_private_set (&g_log_depth, GUINT_TO_POINTER (depth)); - /* had to defer debug initialization until we can keep track of recursion */ - if (!(test_level & G_LOG_FLAG_RECURSION) && !g_debug_initialized) - { - GLogLevelFlags orig_test_level = test_level; - - g_debug_init (); - if ((domain_fatal_mask | g_log_always_fatal) & test_level) - test_level |= G_LOG_FLAG_FATAL; - if (test_level != orig_test_level) - { - /* need a relookup, not nice, but not too bad either */ - g_mutex_lock (&g_messages_lock); - domain = g_log_find_domain_L (log_domain ? log_domain : ""); - log_func = g_log_domain_get_handler_L (domain, test_level, &data); - domain = NULL; - g_mutex_unlock (&g_messages_lock); - } - } if (test_level & G_LOG_FLAG_RECURSION) { diff --git a/glib/gutils.c b/glib/gutils.c index 052126d..fb98e83 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -666,6 +666,7 @@ debug_key_matches (const gchar *key, const gchar *token, guint length) { + /* may not call GLib functions: see note in g_parse_debug_string() */ for (; length; length--, key++, token++) { char k = (*key == '_') ? '-' : tolower (*key ); @@ -708,17 +709,20 @@ g_parse_debug_string (const gchar *string, if (string == NULL) return 0; - /* this function is used by gmem.c/gslice.c initialization code, - * so introducing malloc dependencies here would require adaptions - * of those code portions. + /* this function is used during the initialisation of gmessages, gmem + * and gslice, so it may not do anything that causes memory to be + * allocated or risks messages being emitted. + * + * this means, more or less, that this code may not call anything + * inside GLib. */ - - if (!g_ascii_strcasecmp (string, "all")) + + if (!strcasecmp (string, "all")) { for (i=0; i