profile parser: firm up and document semantics

Clean up the profile parser, firming up the semantics of exactly what
happens in all cases.  Document it.

There are now no more cases of aborting due to failures caused while
opening, parsing or initialising the profile.  Lines of arbitrary length
are also supported.

5 files changed, 126 insertions, 31 deletions

diff --git a/engine/dconf-engine-profile.c b/engine/dconf-engine-profile.c
index a804b30..b499682 100644
--- a/engine/dconf-engine-profile.c
+++ b/engine/dconf-engine-profile.c
@@ -23,11 +23,60 @@
 #include "dconf-engine.h"
 
 #include <string.h>
-#include <stdlib.h>
 #include <stdio.h>
 
 #include "dconf-engine-source.h"
 
+/* This comment attempts to document the exact semantics of
+ * profile-loading.
+ *
+ * In no situation should the result of profile loading be an abort.
+ * There must be a defined outcome for all possible situations.
+ * Warnings may be issued to stderr, however.
+ *
+ * The first step is to determine what profile is to be used.  If a
+ * profile is explicitly specified by the API then it has the top
+ * priority.  Otherwise, if the DCONF_PROFILE environment variable is
+ * set, it takes next priority.
+ *
+ * In both of those cases, if the named profile starts with a slash
+ * character then it is taken to be an absolute pathname.  If it does
+ * not start with a slash then it is assumed to specify a profile file
+ * relative to /etc/dconf/profile/ (ie: DCONF_PROFILE=test for profile
+ * file /etc/dconf/profile/test).
+ *
+ * If opening the profile file fails then the null profile is used.
+ * This is a profile that contains zero sources.  All keys will be
+ * unwritable and all reads will return NULL.
+ *
+ * In the case that no explicit profile was given and DCONF_PROFILE is
+ * unset, dconf attempts to open and use a profile called "user" (ie:
+ * profile file /etc/dconf/profile/user).  If that fails then the
+ * fallback is to act as if the profile file existed and contained a
+ * single line: "user-db:user".
+ *
+ * Note that the fallback case for a missing profile file is different
+ * in the case where a profile was explicitly specified (either by the
+ * API or the environment) and the case where one was not.
+ *
+ * Once a profile file is opened, each line is treated as a possible
+ * source.  Comments and empty lines are ignored.
+ *
+ * All valid source specification lines need to start with 'user-db:' or
+ * 'system-db:'.  If a line doesn't start with one of these then it gets
+ * ignored.  If all the lines in the file get ignored then the result is
+ * effectively the null profile.
+ *
+ * If the first source is a "user-db:" then the resulting profile will
+ * be writable.  No profile starting with a "system-db:" source can ever
+ * be writable.
+ *
+ * Note: even if the source fails to initialise (due to a missing file,
+ * for example) it will remain in the source list.  This could have a
+ * performance cost: in the case of a system-db, for example, dconf will
+ * check if the file has come into existence on every read.
+ */
+
 static DConfEngineSource **
 dconf_engine_null_profile (gint *n_sources)
 {
@@ -42,12 +91,37 @@ dconf_engine_default_profile (gint *n_sources)
   DConfEngineSource **sources;
 
   sources = g_new (DConfEngineSource *, 1);
-  sources[0] = dconf_engine_source_new ("user-db:user");
+  sources[0] = dconf_engine_source_new_default ();
   *n_sources = 1;
 
   return sources;
 }
 
+static DConfEngineSource *
+dconf_engine_profile_handle_line (gchar *line)
+{
+  gchar *end;
+
+  /* remove whitespace at the front */
+  while (g_ascii_isspace (*line))
+    line++;
+
+  /* find the end of the line (or start of comments) */
+  end = line + strcspn (line, "#\n");
+
+  /* remove whitespace at the end */
+  while (end > line && g_ascii_isspace (end[-1]))
+    end--;
+
+  /* if we're left with nothing, return NULL */
+  if (line == end)
+    return NULL;
+
+  *end = '\0';
+
+  return dconf_engine_source_new (line);
+}
+
 static DConfEngineSource **
 dconf_engine_read_profile_file (FILE *file,
                                 gint *n_sources)
@@ -56,34 +130,31 @@ dconf_engine_read_profile_file (FILE *file,
   gchar line[80];
   gint n = 0, a;
 
-  sources = g_new (DConfEngineSource *, (a = 16));
+  sources = g_new (DConfEngineSource *, (a = 4));
 
   while (fgets (line, sizeof line, file))
     {
       DConfEngineSource *source;
-      gchar *end;
-
-      end = strchr (line, '\n');
 
-      if (end == NULL)
+      /* The input file has long lines. */
+      if G_UNLIKELY (!strchr (line, '\n'))
         {
-          g_warning ("ignoring long or unterminated line in dconf profile");
-
-          /* skip until we find the newline or EOF */
-          while (fgets (line, sizeof line, file) && !strchr (line, '\n'));
-
-          continue;
+          GString *long_line;
+
+          long_line = g_string_new (line);
+          while (fgets (line, sizeof line, file))
+            {
+              g_string_append (long_line, line);
+              if (strchr (line, '\n'))
+                break;
+            }
+
+          source = dconf_engine_profile_handle_line (long_line->str);
+          g_string_free (long_line, TRUE);
         }
 
-      if (end == line)
-        continue;
-
-      if (line[0] == '#')
-        continue;
-
-      *end = '\0';
-
-      source = dconf_engine_source_new (line);
+      else
+        source = dconf_engine_profile_handle_line (line);
 
       if (source != NULL)
         {
@@ -96,22 +167,30 @@ dconf_engine_read_profile_file (FILE *file,
 
   *n_sources = n;
 
-  return sources;
+  return g_realloc_n (sources, n, sizeof (DConfEngineSource *));
 }
 
 DConfEngineSource **
-dconf_engine_profile_get_default (gint *n_sources)
+dconf_engine_profile_open (const gchar *profile,
+                           gint        *n_sources)
 {
   DConfEngineSource **sources;
-  const gchar *profile;
   FILE *file;
 
-  profile = getenv ("DCONF_PROFILE");
+  if (profile == NULL)
+    profile = g_getenv ("DCONF_PROFILE");
 
   if (profile == NULL)
-    return dconf_engine_default_profile (n_sources);
+    {
+      file = fopen ("/etc/dconf/profile/user", "r");
 
-  if (profile[0] != '/')
+      /* Only in the case that no profile was specified do we use this
+       * fallback.
+       */
+      if (file == NULL)
+        return dconf_engine_default_profile (n_sources);
+    }
+  else if (profile[0] != '/')
     {
       gchar *filename = g_build_filename ("/etc/dconf/profile", profile, NULL);
       file = fopen (filename, "r");
diff --git a/engine/dconf-engine-profile.h b/engine/dconf-engine-profile.h
index 19935e8..fb04e15 100644
--- a/engine/dconf-engine-profile.h
+++ b/engine/dconf-engine-profile.h
@@ -26,6 +26,7 @@
 #include "dconf-engine-source.h"
 
 G_GNUC_INTERNAL
-DConfEngineSource **    dconf_engine_profile_get_default                (gint *n_sources);
+DConfEngineSource **    dconf_engine_profile_open                       (const gchar *profile,
+                                                                         gint        *n_sources);
 
 #endif
diff --git a/engine/dconf-engine-source.c b/engine/dconf-engine-source.c
index bf080da..2d0e2bd 100644
--- a/engine/dconf-engine-source.c
+++ b/engine/dconf-engine-source.c
@@ -86,7 +86,7 @@ dconf_engine_source_new (const gchar *description)
       break;
 
     default:
-      g_critical ("unknown dconf database description: %s", description);
+      g_warning ("unknown dconf database description: %s", description);
       return NULL;
     }
 
@@ -98,3 +98,15 @@ dconf_engine_source_new (const gchar *description)
 
   return source;
 }
+
+DConfEngineSource *
+dconf_engine_source_new_default (void)
+{
+  DConfEngineSource *source;
+
+  source = g_malloc0 (dconf_engine_source_user_vtable.instance_size);
+  source->vtable = &dconf_engine_source_user_vtable;
+  source->name = g_strdup ("user");
+
+  return source;
+}
diff --git a/engine/dconf-engine-source.h b/engine/dconf-engine-source.h
index 6d51bc7..cb8e9c2 100644
--- a/engine/dconf-engine-source.h
+++ b/engine/dconf-engine-source.h
@@ -62,6 +62,9 @@ G_GNUC_INTERNAL
 DConfEngineSource *     dconf_engine_source_new                         (const gchar        *name);
 
 G_GNUC_INTERNAL
+DConfEngineSource *     dconf_engine_source_new_default                 (void);
+
+G_GNUC_INTERNAL
 gboolean                dconf_engine_source_init                        (DConfEngineSource  *source);
 
 #endif /* __dconf_engine_source_h__ */
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index 3c9d9f5..1e9aa64 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -234,7 +234,7 @@ dconf_engine_new (gpointer       user_data,
   g_mutex_init (&engine->sources_lock);
   g_mutex_init (&engine->queue_lock);
 
-  engine->sources = dconf_engine_profile_get_default (&engine->n_sources);
+  engine->sources = dconf_engine_profile_open (NULL, &engine->n_sources);
 
   for (i = 0; i < engine->n_sources; i++)
     if (!dconf_engine_source_init (engine->sources[i]))