Libguestfs - Feb 2014 - [PATCH 1/2] fish: small refactor of config reading code

Even though so far there is just one possible setting to read, isolate
in an own function the code to parse a configuration file and read the
settings out of it.

Now there's a new config_t handle used every time, but since config_read
would reset an handle completely, there is no behaviour change.
---
 fish/config.c | 88 +++++++++++++++++++++--------------------------------------
 1 file changed, 31 insertions(+), 57 deletions(-)

diff --git a/fish/config.c b/fish/config.c
index e9f437a..02d850b 100644
--- a/fish/config.c
+++ b/fish/config.c
@@ -42,87 +42,61 @@ static const char *etc_filename =
"/etc/libguestfs-tools.conf";
  * global handle 'g' is opened.
  */
 
-void
-parse_config (void)
+static void
+read_config_from_file (const char *filename)
 {
-  const char *home;
   FILE *fp;
-  config_t conf;
-
-  config_init (&conf);
-
-  /* Try $HOME first. */
-  home = getenv ("HOME");
-  if (home != NULL) {
-    CLEANUP_FREE char *path = NULL;
 
-    if (asprintf (&path, "%s/%s", home, home_filename) == -1) {
-      perror ("asprintf");
-      exit (EXIT_FAILURE);
-    }
+  fp = fopen (filename, "r");
+  if (fp != NULL) {
+    config_t conf;
 
-    fp = fopen (path, "r");
-    if (fp != NULL) {
-      /*
-      if (verbose)
-        fprintf (stderr, "%s: reading configuration from %s\n",
-                 program_name, path);
-      */
-
-      if (config_read (&conf, fp) == CONFIG_FALSE) {
-        fprintf (stderr,
-                 _("%s: %s: line %d: error parsing configuration file:
%s\n"),
-                 program_name, path, config_error_line (&conf),
-                 config_error_text (&conf));
-        exit (EXIT_FAILURE);
-      }
-
-      if (fclose (fp) == -1) {
-        perror (path);
-        exit (EXIT_FAILURE);
-      }
-
-      /* Notes:
-       *
-       * (1) It's not obvious from the documentation, that config_read
-       * completely resets the 'conf' structure.  This means we cannot
-       * call config_read twice on the two possible configuration
-       * files, but instead have to copy out settings into our
-       * variables between calls.
-       *
-       * (2) If the next call fails then 'read_only' variable is not
-       * updated.  Failure could happen just because the setting is
-       * missing from the configuration file, so we ignore it here.
-       */
-      config_lookup_bool (&conf, "read_only", &read_only);
-    }
-  }
+    config_init (&conf);
 
-  fp = fopen (etc_filename, "r");
-  if (fp != NULL) {
     /*
     if (verbose)
       fprintf (stderr, "%s: reading configuration from %s\n",
-               program_name, etc_filename);
+               program_name, filename);
     */
 
     if (config_read (&conf, fp) == CONFIG_FALSE) {
       fprintf (stderr,
                _("%s: %s: line %d: error parsing configuration file:
%s\n"),
-               program_name, etc_filename, config_error_line (&conf),
+               program_name, filename, config_error_line (&conf),
                config_error_text (&conf));
       exit (EXIT_FAILURE);
     }
 
     if (fclose (fp) == -1) {
-      perror (etc_filename);
+      perror (filename);
       exit (EXIT_FAILURE);
     }
 
     config_lookup_bool (&conf, "read_only", &read_only);
+
+    config_destroy (&conf);
+  }
+}
+
+void
+parse_config (void)
+{
+  const char *home;
+
+  /* Try $HOME first. */
+  home = getenv ("HOME");
+  if (home != NULL) {
+    CLEANUP_FREE char *path = NULL;
+
+    if (asprintf (&path, "%s/%s", home, home_filename) == -1) {
+      perror ("asprintf");
+      exit (EXIT_FAILURE);
+    }
+
+    read_config_from_file (path);
   }
 
-  config_destroy (&conf);
+  read_config_from_file (etc_filename);
 }
 
 #else /* !HAVE_LIBCONFIG */
-- 
1.8.3.1

First read the global configuration and then the local one in user's
HOME, so the latter can really override system settings.
---
 fish/config.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fish/config.c b/fish/config.c
index 02d850b..9e5da87 100644
--- a/fish/config.c
+++ b/fish/config.c
@@ -83,7 +83,10 @@ parse_config (void)
 {
   const char *home;
 
-  /* Try $HOME first. */
+  /* Try the global configuration first. */
+  read_config_from_file (etc_filename);
+
+  /* Read the configuration from $HOME, to override system settings. */
   home = getenv ("HOME");
   if (home != NULL) {
     CLEANUP_FREE char *path = NULL;
@@ -95,8 +98,6 @@ parse_config (void)
 
     read_config_from_file (path);
   }
-
-  read_config_from_file (etc_filename);
 }
 
 #else /* !HAVE_LIBCONFIG */
-- 
1.8.3.1

On Mon, Feb 17, 2014 at 05:23:11PM +0100, Pino Toscano
wrote:
> First read the global configuration and then the local one in user's
> HOME, so the latter can really override system settings.
> ---
>  fish/config.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fish/config.c b/fish/config.c
> index 02d850b..9e5da87 100644
> --- a/fish/config.c
> +++ b/fish/config.c
> @@ -83,7 +83,10 @@ parse_config (void)
>  {
>    const char *home;
>  
> -  /* Try $HOME first. */
> +  /* Try the global configuration first. */
> +  read_config_from_file (etc_filename);
> +
> +  /* Read the configuration from $HOME, to override system settings. */
>    home = getenv ("HOME");
>    if (home != NULL) {
>      CLEANUP_FREE char *path = NULL;
> @@ -95,8 +98,6 @@ parse_config (void)
>  
>      read_config_from_file (path);
>    }
> -
> -  read_config_from_file (etc_filename);
>  }
>  
>  #else /* !HAVE_LIBCONFIG */
ACK to both.

Thanks, Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)