From 067da211cd63a62cc03dc2745d40b9aa0e64dabc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Sep 2017 09:31:03 -0400 Subject: [PATCH] lib/syslinux: Port to new code style 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 | 156 ++++++++------------- 1 file changed, 56 insertions(+), 100 deletions(-) diff --git a/src/libostree/ostree-bootloader-syslinux.c b/src/libostree/ostree-bootloader-syslinux.c index 05cb173eec..16f287b9c9 100644 --- a/src/libostree/ostree-bootloader-syslinux.c +++ b/src/libostree/ostree-bootloader-syslinux.c @@ -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