lib/syslinux: Port to new code style
authorColin Walters <walters@verbum.org>
Fri, 8 Sep 2017 13:31:03 +0000 (09:31 -0400)
committerAtomic Bot <atomic-devel@projectatomic.io>
Fri, 8 Sep 2017 18:00:19 +0000 (18:00 +0000)
There was only one tricky bit here around the ownership of the lines; I made use
of `g_steal_pointer()` to consistently track ownership, and converted to a `for`
loop while still preserving the loop logic around the last entry.

Closes: #1154
Approved by: jlebon

src/libostree/ostree-bootloader-syslinux.c

index 05cb173eec2d8c2ed0b8ac90487b2fdb8ec5ea6b..16f287b9c935dce3a17d929c739dc776fc1be1bc 100644 (file)
@@ -59,44 +59,33 @@ _ostree_bootloader_syslinux_get_name (OstreeBootloader *bootloader)
 }
 
 static gboolean
-append_config_from_boostree_loader_entries (OstreeBootloaderSyslinux  *self,
-                                        gboolean               regenerate_default,
-                                        int                    bootversion,
-                                        GPtrArray             *new_lines,
-                                        GCancellable          *cancellable,
-                                        GError               **error)
+append_config_from_loader_entries (OstreeBootloaderSyslinux  *self,
+                                   gboolean               regenerate_default,
+                                   int                    bootversion,
+                                   GPtrArray             *new_lines,
+                                   GCancellable          *cancellable,
+                                   GError               **error)
 {
-  gboolean ret = FALSE;
-  g_autoptr(GPtrArray) boostree_loader_configs = NULL;
-  guint i;
-
-  if (!_ostree_sysroot_read_boot_loader_configs (self->sysroot, bootversion, &boostree_loader_configs,
+  g_autoptr(GPtrArray) loader_configs = NULL;
+  if (!_ostree_sysroot_read_boot_loader_configs (self->sysroot, bootversion, &loader_configs,
                                                  cancellable, error))
-    goto out;
+    return FALSE;
 
-  for (i = 0; i < boostree_loader_configs->len; i++)
+  for (guint i = 0; i < loader_configs->len; i++)
     {
-      OstreeBootconfigParser *config = boostree_loader_configs->pdata[i];
-      const char *val;
-
-      val = ostree_bootconfig_parser_get (config, "title");
+      OstreeBootconfigParser *config = loader_configs->pdata[i];
+      const char *val = ostree_bootconfig_parser_get (config, "title");
       if (!val)
         val = "(Untitled)";
 
       if (regenerate_default && i == 0)
-        {
-          g_ptr_array_add (new_lines, g_strdup_printf ("DEFAULT %s", val));
-        }
+        g_ptr_array_add (new_lines, g_strdup_printf ("DEFAULT %s", val));
 
       g_ptr_array_add (new_lines, g_strdup_printf ("LABEL %s", val));
-      
+
       val = ostree_bootconfig_parser_get (config, "linux");
       if (!val)
-        {
-          g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                       "No \"linux\" key in bootloader config");
-          goto out;
-        }
+        return glnx_throw (error, "No \"linux\" key in bootloader config");
       g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s", val));
 
       val = ostree_bootconfig_parser_get (config, "initrd");
@@ -108,72 +97,57 @@ append_config_from_boostree_loader_entries (OstreeBootloaderSyslinux  *self,
         g_ptr_array_add (new_lines, g_strdup_printf ("\tAPPEND %s", val));
     }
 
-  ret = TRUE;
- out:
-  return ret;
+  return TRUE;
 }
 
 static gboolean
 _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
-                                     int                    bootversion,
-                                     GCancellable          *cancellable,
-                                     GError               **error)
+                                          int                    bootversion,
+                                          GCancellable          *cancellable,
+                                          GError               **error)
 {
-  gboolean ret = FALSE;
   OstreeBootloaderSyslinux *self = OSTREE_BOOTLOADER_SYSLINUX (bootloader);
-  g_autoptr(GFile) new_config_path = NULL;
-  g_autofree char *config_contents = NULL;
-  g_autofree char *new_config_contents = NULL;
-  g_autoptr(GPtrArray) new_lines = NULL;
-  g_autoptr(GPtrArray) tmp_lines = NULL;
-  g_autofree char *kernel_arg = NULL;
-  gboolean saw_default = FALSE;
-  gboolean regenerate_default = FALSE;
-  gboolean parsing_label = FALSE;
-  char **lines = NULL;
-  char **iter;
-  guint i;
 
-  new_config_path = ot_gfile_resolve_path_printf (self->sysroot->path, "boot/loader.%d/syslinux.cfg",
-                                                  bootversion);
+  g_autoptr(GFile) new_config_path =
+    ot_gfile_resolve_path_printf (self->sysroot->path, "boot/loader.%d/syslinux.cfg", bootversion);
 
   /* This should follow the symbolic link to the current bootversion. */
-  config_contents = glnx_file_get_contents_utf8_at (AT_FDCWD, gs_file_get_path_cached (self->config_path), NULL,
-                                                    cancellable, error);
+  g_autofree char *config_contents =
+    glnx_file_get_contents_utf8_at (AT_FDCWD, gs_file_get_path_cached (self->config_path), NULL,
+                                    cancellable, error);
   if (!config_contents)
-    goto out;
+    return FALSE;
 
-  lines = g_strsplit (config_contents, "\n", -1);
-  new_lines = g_ptr_array_new_with_free_func (g_free);
-  tmp_lines = g_ptr_array_new_with_free_func (g_free);
-  
+  g_auto(GStrv) lines = g_strsplit (config_contents, "\n", -1);
+  g_autoptr(GPtrArray) new_lines = g_ptr_array_new_with_free_func (g_free);
+  g_autoptr(GPtrArray) tmp_lines = g_ptr_array_new_with_free_func (g_free);
+
+  g_autofree char *kernel_arg = NULL;
+  gboolean saw_default = FALSE;
+  gboolean regenerate_default = FALSE;
+  gboolean parsing_label = FALSE;
   /* Note special iteration condition here; we want to also loop one
    * more time at the end where line = NULL to ensure we finish off
    * processing the last LABEL.
    */
-  iter = lines;
-  while (TRUE)
+  for (char **iter = lines; iter; iter++)
     {
-      char *line = *iter;
+      const char *line = *iter;
       gboolean skip = FALSE;
 
-      if (parsing_label && 
+      if (parsing_label &&
           (line == NULL || !g_str_has_prefix (line, "\t")))
         {
           parsing_label = FALSE;
           if (kernel_arg == NULL)
-            {
-              g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                                   "No KERNEL argument found after LABEL");
-              goto out;
-            }
+            return glnx_throw (error, "No KERNEL argument found after LABEL");
 
           /* If this is a non-ostree kernel, just emit the lines
            * we saw.
            */
           if (!g_str_has_prefix (kernel_arg, "/ostree/"))
             {
-              for (i = 0; i < tmp_lines->len; i++)
+              for (guint i = 0; i < tmp_lines->len; i++)
                 {
                   g_ptr_array_add (new_lines, tmp_lines->pdata[i]);
                   tmp_lines->pdata[i] = NULL; /* Transfer ownership */
@@ -211,55 +185,37 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader          *bootloader,
            *     the title to hopefully avoid regressions. */
           if (g_str_has_prefix (line, "DEFAULT ostree:") ||  /* old format */
               strstr (line, "(ostree") != NULL)              /* new format */
-            {
-              regenerate_default = TRUE;
-            }
+            regenerate_default = TRUE;
           skip = TRUE;
         }
-      
-      if (skip)
-        {
-          g_free (line);
-        }
-      else
+
+      if (!skip)
         {
           if (parsing_label)
-            {
-              g_ptr_array_add (tmp_lines, line);
-            }
+            g_ptr_array_add (tmp_lines, g_strdup (line));
           else
-            {
-              g_ptr_array_add (new_lines, line);
-            }
+            g_ptr_array_add (new_lines, g_strdup (line));
         }
-      /* Transfer ownership */
-      *iter = NULL;
-      iter++;
     }
 
   if (!saw_default)
     regenerate_default = TRUE;
 
-  if (!append_config_from_boostree_loader_entries (self, regenerate_default,
-                                               bootversion, new_lines,
-                                               cancellable, error))
-    goto out;
+  if (!append_config_from_loader_entries (self, regenerate_default,
+                                          bootversion, new_lines,
+                                          cancellable, error))
+    return FALSE;
 
-  new_config_contents = _ostree_sysroot_join_lines (new_lines);
-  {
-    g_autoptr(GBytes) new_config_contents_bytes =
-      g_bytes_new_static (new_config_contents,
-                          strlen (new_config_contents));
+  g_autofree char *new_config_contents = _ostree_sysroot_join_lines (new_lines);
+  g_autoptr(GBytes) new_config_contents_bytes =
+    g_bytes_new_static (new_config_contents,
+                        strlen (new_config_contents));
 
-    if (!ot_gfile_replace_contents_fsync (new_config_path, new_config_contents_bytes,
-                                          cancellable, error))
-      goto out;
-  }
-  
-  ret = TRUE;
- out:
-  g_free (lines); /* Note we freed elements individually */
-  return ret;
+  if (!ot_gfile_replace_contents_fsync (new_config_path, new_config_contents_bytes,
+                                        cancellable, error))
+    return FALSE;
+
+  return TRUE;
 }
 
 static void