[glib] Clean up process of calling g_debug_init()

[Ryan Lortie <ryanl src gnome org>]

commit 310c3ed4cc733f47b88b44fb03757bd7213a4f9a
Author: Ryan Lortie <desrt desrt ca>
Date:   Wed Sep 21 21:20:07 2011 -0400

    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 |   87 +++++++++++++++++++++--------------------------------
 glib/gutils.c    |   16 ++++++----
 2 files changed, 45 insertions(+), 58 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<nkeys; i++)
 	result |= keys[i].value;
     }
-  else if (!g_ascii_strcasecmp (string, "help"))
+  else if (!strcasecmp (string, "help"))
     {
       /* using stdio directly for the reason stated above */
       fprintf (stderr, "Supported debug values: ");