Refactor: sysroot.bootloader: Store enum value rather than string

It's easier to extend and it centralises the config parsing.  In other
places we will no longer need to use `g_str_equal` to match these values,
a `switch` statement will be sufficient.

diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
index cbbe69717dc22f8a4a99bc15ded0c40bbe4fb78a..14e2f753d8e7184196b213254ad5eb0dab56313f 100644 (file)
--- a/src/libostree/ostree-repo-private.h
+++ b/src/libostree/ostree-repo-private.h
@@ -110,6 +110,22 @@ typedef enum {
   _OSTREE_FEATURE_YES,
 } _OstreeFeatureSupport;
 
+/* Possible values for the sysroot.bootloader configuration variable */
+typedef enum {
+  CFG_SYSROOT_BOOTLOADER_OPT_AUTO = 0,
+  CFG_SYSROOT_BOOTLOADER_OPT_NONE,
+  CFG_SYSROOT_BOOTLOADER_OPT_ZIPL,
+  /* Non-exhaustive */
+} OstreeCfgSysrootBootloaderOpt;
+
+static const char* const CFG_SYSROOT_BOOTLOADER_OPTS_STR[] = {
+  /* This must be kept in the same order as the enum */
+  "auto",
+  "none",
+  "zipl",
+  NULL,
+};
+
 /**
  * OstreeRepo:
  *
@@ -193,7 +209,7 @@ struct OstreeRepo {
   guint64 payload_link_threshold;
   gint fs_support_reflink; /* The underlying filesystem has support for ioctl (FICLONE..) */
   gchar **repo_finders;
-  gchar *bootloader; /* Configure which bootloader to use. */
+  OstreeCfgSysrootBootloaderOpt bootloader; /* Configure which bootloader to use. */
 
   OstreeRepo *parent_repo;
 };

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index d3066e6a3522235469af493e59521f369e6e294e..11a209a418f0904e97b6451d8f101ff4c9cf75e5 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -1048,7 +1048,6 @@ ostree_repo_finalize (GObject *object)
   g_mutex_clear (&self->txn_lock);
   g_free (self->collection_id);
   g_strfreev (self->repo_finders);
-  g_free (self->bootloader);
 
   g_clear_pointer (&self->remotes, g_hash_table_destroy);
   g_mutex_clear (&self->remotes_lock);
@@ -3186,28 +3185,28 @@ reload_sysroot_config (OstreeRepo          *self,
                        GCancellable        *cancellable,
                        GError             **error)
 {
-  { g_autofree char *bootloader = NULL;
+  g_autofree char *bootloader = NULL;
 
-    if (!ot_keyfile_get_value_with_default_group_optional (self->config, "sysroot",
-                                                           "bootloader", "auto",
-                                                           &bootloader, error))
-      return FALSE;
-
-    /* TODO: possibly later add support for specifying a generic bootloader
-     * binary "x" in /usr/lib/ostree/bootloaders/x). See:
-     * https://github.com/ostreedev/ostree/issues/1719
-     * https://github.com/ostreedev/ostree/issues/1801
-     * Also, dedup these strings with the bootloader implementations
-     */
-    if (!(g_str_equal (bootloader, "auto") || g_str_equal (bootloader, "none")
-          || g_str_equal (bootloader, "zipl")))
-      return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader);
+  if (!ot_keyfile_get_value_with_default_group_optional (self->config, "sysroot",
+                                                         "bootloader", "auto",
+                                                         &bootloader, error))
+    return FALSE;
 
-    g_free (self->bootloader);
-    self->bootloader = g_steal_pointer (&bootloader);
-  }
+  /* TODO: possibly later add support for specifying a generic bootloader
+   * binary "x" in /usr/lib/ostree/bootloaders/x). See:
+   * https://github.com/ostreedev/ostree/issues/1719
+   * https://github.com/ostreedev/ostree/issues/1801
+   */
+  for (int i = 0; CFG_SYSROOT_BOOTLOADER_OPTS_STR[i]; i++)
+    {
+      if (g_str_equal (bootloader, CFG_SYSROOT_BOOTLOADER_OPTS_STR[i]))
+        {
+          self->bootloader = (OstreeCfgSysrootBootloaderOpt) i;
+          return TRUE;
+        }
+    }
 
-  return TRUE;
+  return glnx_throw (error, "Invalid bootloader configuration: '%s'", bootloader);
 }
 
 /**
@@ -6331,7 +6330,7 @@ ostree_repo_get_bootloader (OstreeRepo   *self)
 {
   g_return_val_if_fail (OSTREE_IS_REPO (self), NULL);
 
-  return self->bootloader;
+  return CFG_SYSROOT_BOOTLOADER_OPTS_STR[self->bootloader];
 }
 
 

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index f183edd1cf94461058cfb3a1be100585c9370e7f..63065fb95eb97975a7a1519e552ae39116c9690f 100644 (file)
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -1340,50 +1340,53 @@ _ostree_sysroot_query_bootloader (OstreeSysroot     *sysroot,
                                   GError           **error)
 {
   OstreeRepo *repo = ostree_sysroot_repo (sysroot);
-  g_autofree gchar *bootloader_config = ostree_repo_get_bootloader (repo);
+  OstreeCfgSysrootBootloaderOpt bootloader_config = repo->bootloader;
 
-  g_debug ("Using bootloader configuration: %s", bootloader_config);
+  g_debug ("Using bootloader configuration: %s",
+      CFG_SYSROOT_BOOTLOADER_OPTS_STR[bootloader_config]);
 
   g_autoptr(OstreeBootloader) ret_loader = NULL;
-  if (g_str_equal (bootloader_config, "none"))
+  switch (repo->bootloader)
     {
+    case CFG_SYSROOT_BOOTLOADER_OPT_NONE:
       /* No bootloader specified; do not query bootloaders to run. */
       ret_loader = NULL;
-    }
-  else if (g_str_equal (bootloader_config, "zipl"))
-    {
+      break;
+    case CFG_SYSROOT_BOOTLOADER_OPT_ZIPL:
       /* We never consider zipl as active by default, so it can only be created
        * if it's explicitly requested in the config */
       ret_loader = (OstreeBootloader*) _ostree_bootloader_zipl_new (sysroot);
-    }
-  else if (g_str_equal (bootloader_config, "auto"))
-    {
-      gboolean is_active;
-      ret_loader = (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot);
-      if (!_ostree_bootloader_query (ret_loader, &is_active,
-                                     cancellable, error))
-        return FALSE;
+      break;
+    case CFG_SYSROOT_BOOTLOADER_OPT_AUTO:
+      {
+        gboolean is_active;
+        ret_loader = (OstreeBootloader*)_ostree_bootloader_syslinux_new (sysroot);
+        if (!_ostree_bootloader_query (ret_loader, &is_active,
+                                      cancellable, error))
+          return FALSE;
 
-      if (!is_active)
-        {
-          g_object_unref (ret_loader);
-          ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot);
-          if (!_ostree_bootloader_query (ret_loader, &is_active,
-                                         cancellable, error))
-            return FALSE;
-        }
-      if (!is_active)
-        {
-          g_object_unref (ret_loader);
-          ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot);
-          if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error))
-            return FALSE;
-        }
-      if (!is_active)
-        g_clear_object (&ret_loader);
+        if (!is_active)
+          {
+            g_object_unref (ret_loader);
+            ret_loader = (OstreeBootloader*)_ostree_bootloader_grub2_new (sysroot);
+            if (!_ostree_bootloader_query (ret_loader, &is_active,
+                                          cancellable, error))
+              return FALSE;
+          }
+        if (!is_active)
+          {
+            g_object_unref (ret_loader);
+            ret_loader = (OstreeBootloader*)_ostree_bootloader_uboot_new (sysroot);
+            if (!_ostree_bootloader_query (ret_loader, &is_active, cancellable, error))
+              return FALSE;
+          }
+        if (!is_active)
+          g_clear_object (&ret_loader);
+        break;
+      }
+    default:
+      g_assert_not_reached ();
     }
-  else
-    g_assert_not_reached ();
 
   ot_transfer_out_value(out_bootloader, &ret_loader);
   return TRUE;