From 141a79f491fd4bf5ea0d66039065c9f9649bfc0e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 15 Feb 2014 18:08:59 -0500 Subject: [PATCH 1/1] Extract looping over /proc/cmdline into a shared function In cryptsetup-generator automatic cleanup had to be replaced with manual cleanup, and the code gets a bit longer. But existing code had the issue that it returned negative values from main(), which was wrong, so should be reworked anyway. --- src/core/main.c | 31 +-- src/cryptsetup/cryptsetup-generator.c | 260 +++++++++++++------------- src/fsck/fsck.c | 48 ++--- src/fstab-generator/fstab-generator.c | 49 ++--- src/modules-load/modules-load.c | 38 +--- src/quotacheck/quotacheck.c | 43 ++--- src/shared/util.c | 29 +++ src/shared/util.h | 1 + 8 files changed, 216 insertions(+), 283 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 14e21d634..fc85eedfe 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -694,35 +694,6 @@ static int parse_config_file(void) { return 0; } -static int parse_proc_cmdline(void) { - _cleanup_free_ char *line = NULL; - char *w, *state; - size_t l; - int r; - - r = proc_cmdline(&line); - if (r < 0) - log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); - if (r <= 0) - return 0; - - FOREACH_WORD_QUOTED(w, l, line, state) { - _cleanup_free_ char *word; - - word = strndup(w, l); - if (!word) - return log_oom(); - - r = parse_proc_cmdline_word(word); - if (r < 0) { - log_error("Failed on cmdline argument %s: %s", word, strerror(-r)); - return r; - } - } - - return 0; -} - static int parse_argv(int argc, char *argv[]) { enum { @@ -1408,7 +1379,7 @@ int main(int argc, char *argv[]) { goto finish; if (arg_running_as == SYSTEMD_SYSTEM) - if (parse_proc_cmdline() < 0) + if (parse_proc_cmdline(parse_proc_cmdline_word) < 0) goto finish; log_parse_environment(); diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index 46ad9b8d6..38c746b8b 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -34,6 +34,11 @@ static const char *arg_dest = "/tmp"; static bool arg_enabled = true; static bool arg_read_crypttab = true; +static char **arg_disks; +static char **arg_options; +static char *arg_keyfile; + + static bool has_option(const char *haystack, const char *needle) { const char *f = haystack; size_t l; @@ -250,122 +255,94 @@ static int create_disk( return 0; } -static int parse_proc_cmdline( - char ***arg_proc_cmdline_disks, - char ***arg_proc_cmdline_options, - char **arg_proc_cmdline_keyfile) { - - _cleanup_free_ char *line = NULL; - char *w = NULL, *state = NULL; - size_t l; +static int parse_proc_cmdline_word(const char *word) { int r; - r = proc_cmdline(&line); - if (r < 0) - log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); - if (r <= 0) - return 0; - - FOREACH_WORD_QUOTED(w, l, line, state) { - _cleanup_free_ char *word = NULL; + if (startswith(word, "luks=")) { + r = parse_boolean(word + 5); + if (r < 0) + log_warning("Failed to parse luks switch %s. Ignoring.", word + 5); + else + arg_enabled = r; - word = strndup(w, l); - if (!word) - return log_oom(); + } else if (startswith(word, "rd.luks=")) { - if (startswith(word, "luks=")) { - r = parse_boolean(word + 5); + if (in_initrd()) { + r = parse_boolean(word + 8); if (r < 0) - log_warning("Failed to parse luks switch %s. Ignoring.", word + 5); + log_warning("Failed to parse luks switch %s. Ignoring.", word + 8); else arg_enabled = r; + } - } else if (startswith(word, "rd.luks=")) { + } else if (startswith(word, "luks.crypttab=")) { + r = parse_boolean(word + 14); + if (r < 0) + log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 14); + else + arg_read_crypttab = r; - if (in_initrd()) { - r = parse_boolean(word + 8); - if (r < 0) - log_warning("Failed to parse luks switch %s. Ignoring.", word + 8); - else - arg_enabled = r; - } + } else if (startswith(word, "rd.luks.crypttab=")) { - } else if (startswith(word, "luks.crypttab=")) { - r = parse_boolean(word + 14); + if (in_initrd()) { + r = parse_boolean(word + 17); if (r < 0) - log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 14); + log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 17); else arg_read_crypttab = r; + } - } else if (startswith(word, "rd.luks.crypttab=")) { + } else if (startswith(word, "luks.uuid=")) { + if (strv_extend(&arg_disks, word + 10) < 0) + return log_oom(); - if (in_initrd()) { - r = parse_boolean(word + 17); - if (r < 0) - log_warning("Failed to parse luks crypttab switch %s. Ignoring.", word + 17); - else - arg_read_crypttab = r; - } + } else if (startswith(word, "rd.luks.uuid=")) { - } else if (startswith(word, "luks.uuid=")) { - if (strv_extend(arg_proc_cmdline_disks, word + 10) < 0) + if (in_initrd()) { + if (strv_extend(&arg_disks, word + 13) < 0) return log_oom(); + } - } else if (startswith(word, "rd.luks.uuid=")) { + } else if (startswith(word, "luks.options=")) { + if (strv_extend(&arg_options, word + 13) < 0) + return log_oom(); - if (in_initrd()) { - if (strv_extend(arg_proc_cmdline_disks, word + 13) < 0) - return log_oom(); - } + } else if (startswith(word, "rd.luks.options=")) { - } else if (startswith(word, "luks.options=")) { - if (strv_extend(arg_proc_cmdline_options, word + 13) < 0) + if (in_initrd()) { + if (strv_extend(&arg_options, word + 16) < 0) return log_oom(); + } - } else if (startswith(word, "rd.luks.options=")) { + } else if (startswith(word, "luks.key=")) { + free(arg_keyfile); + arg_keyfile = strdup(word + 9); + if (!arg_keyfile) + return log_oom(); - if (in_initrd()) { - if (strv_extend(arg_proc_cmdline_options, word + 16) < 0) - return log_oom(); - } + } else if (startswith(word, "rd.luks.key=")) { - } else if (startswith(word, "luks.key=")) { - if (*arg_proc_cmdline_keyfile) - free(*arg_proc_cmdline_keyfile); - *arg_proc_cmdline_keyfile = strdup(word + 9); - if (!*arg_proc_cmdline_keyfile) + if (in_initrd()) { + free(arg_keyfile); + arg_keyfile = strdup(word + 12); + if (!arg_keyfile) return log_oom(); + } - } else if (startswith(word, "rd.luks.key=")) { - - if (in_initrd()) { - if (*arg_proc_cmdline_keyfile) - free(*arg_proc_cmdline_keyfile); - *arg_proc_cmdline_keyfile = strdup(word + 12); - if (!*arg_proc_cmdline_keyfile) - return log_oom(); - } - - } else if (startswith(word, "luks.") || - (in_initrd() && startswith(word, "rd.luks."))) { + } else if (startswith(word, "luks.") || + (in_initrd() && startswith(word, "rd.luks."))) { - log_warning("Unknown kernel switch %s. Ignoring.", word); - } + log_warning("Unknown kernel switch %s. Ignoring.", word); } - strv_uniq(*arg_proc_cmdline_disks); - return 0; } int main(int argc, char *argv[]) { - _cleanup_strv_free_ char **arg_proc_cmdline_disks_done = NULL; - _cleanup_strv_free_ char **arg_proc_cmdline_disks = NULL; - _cleanup_strv_free_ char **arg_proc_cmdline_options = NULL; - _cleanup_free_ char *arg_proc_cmdline_keyfile = NULL; + _cleanup_strv_free_ char **disks_done = NULL; _cleanup_fclose_ FILE *f = NULL; unsigned n = 0; - int r = EXIT_SUCCESS; + int r = EXIT_FAILURE, r2 = EXIT_FAILURE; char **i; if (argc > 1 && argc != 4) { @@ -382,11 +359,15 @@ int main(int argc, char *argv[]) { umask(0022); - if (parse_proc_cmdline(&arg_proc_cmdline_disks, &arg_proc_cmdline_options, &arg_proc_cmdline_keyfile) < 0) - return EXIT_FAILURE; + if (parse_proc_cmdline(parse_proc_cmdline_word) < 0) + goto cleanup; - if (!arg_enabled) - return EXIT_SUCCESS; + if (!arg_enabled) { + r = r2 = EXIT_SUCCESS; + goto cleanup; + } + + strv_uniq(arg_disks); if (arg_read_crypttab) { struct stat st; @@ -395,17 +376,14 @@ int main(int argc, char *argv[]) { if (!f) { if (errno == ENOENT) r = EXIT_SUCCESS; - else { - r = EXIT_FAILURE; + else log_error("Failed to open /etc/crypttab: %m"); - } goto next; } if (fstat(fileno(f), &st) < 0) { log_error("Failed to stat /etc/crypttab: %m"); - r = EXIT_FAILURE; goto next; } @@ -433,36 +411,34 @@ int main(int argc, char *argv[]) { k = sscanf(l, "%ms %ms %ms %ms", &name, &device, &password, &options); if (k < 2 || k > 4) { log_error("Failed to parse /etc/crypttab:%u, ignoring.", n); - r = EXIT_FAILURE; continue; } - if (arg_proc_cmdline_options) { - /* - If options are specified on the kernel commandline, let them override - the ones from crypttab. - */ - STRV_FOREACH(i, arg_proc_cmdline_options) { - _cleanup_free_ char *proc_uuid = NULL, *proc_options = NULL; - const char *p = *i; - - k = sscanf(p, "%m[0-9a-fA-F-]=%ms", &proc_uuid, &proc_options); - if (k == 2 && streq(proc_uuid, device + 5)) { - if (options) - free(options); - options = strdup(p); - if (!proc_options) - return log_oom(); + /* + If options are specified on the kernel commandline, let them override + the ones from crypttab. + */ + STRV_FOREACH(i, arg_options) { + _cleanup_free_ char *proc_uuid = NULL, *proc_options = NULL; + const char *p = *i; + + k = sscanf(p, "%m[0-9a-fA-F-]=%ms", &proc_uuid, &proc_options); + if (k == 2 && streq(proc_uuid, device + 5)) { + free(options); + options = strdup(p); + if (!proc_options) { + log_oom(); + goto cleanup; } } } - if (arg_proc_cmdline_disks) { + if (arg_disks) { /* If luks UUIDs are specified on the kernel command line, use them as a filter for /etc/crypttab and only generate units for those. */ - STRV_FOREACH(i, arg_proc_cmdline_disks) { + STRV_FOREACH(i, arg_disks) { _cleanup_free_ char *proc_device = NULL, *proc_name = NULL; const char *p = *i; @@ -472,26 +448,31 @@ int main(int argc, char *argv[]) { proc_name = strappend("luks-", p); proc_device = strappend("UUID=", p); - if (!proc_name || !proc_device) - return log_oom(); + if (!proc_name || !proc_device) { + log_oom(); + goto cleanup; + } if (streq(proc_device, device) || streq(proc_name, name)) { if (create_disk(name, device, password, options) < 0) - r = EXIT_FAILURE; + goto cleanup; - if (strv_extend(&arg_proc_cmdline_disks_done, p) < 0) - return log_oom(); + if (strv_extend(&disks_done, p) < 0) { + log_oom(); + goto cleanup; + } } } - } else { - if (create_disk(name, device, password, options) < 0) - r = EXIT_FAILURE; - } + } else if (create_disk(name, device, password, options) < 0) + goto cleanup; + } } + r = EXIT_SUCCESS; + next: - STRV_FOREACH(i, arg_proc_cmdline_disks) { + STRV_FOREACH(i, arg_disks) { /* Generate units for those UUIDs, which were specified on the kernel command line and not yet written. @@ -503,22 +484,24 @@ next: if (startswith(p, "luks-")) p += 5; - if (strv_contains(arg_proc_cmdline_disks_done, p)) + if (strv_contains(disks_done, p)) continue; name = strappend("luks-", p); device = strappend("UUID=", p); - if (!name || !device) - return log_oom(); + if (!name || !device) { + log_oom(); + goto cleanup; + } - if (arg_proc_cmdline_options) { + if (arg_options) { /* If options are specified on the kernel commandline, use them. */ char **j; - STRV_FOREACH(j, arg_proc_cmdline_options) { + STRV_FOREACH(j, arg_options) { _cleanup_free_ char *proc_uuid = NULL, *proc_options = NULL; const char *s = *j; int k; @@ -529,29 +512,42 @@ next: if (options) free(options); options = strdup(proc_options); - if (!options) - return log_oom(); + if (!options) { + log_oom(); + goto cleanup; + } } } else if (!options) { /* Fall back to options without a specified UUID */ options = strdup(s); - if (!options) - return log_oom(); + if (!options) { + log_oom(); + goto cleanup; + }; } } } if (!options) { options = strdup("timeout=0"); - if (!options) - return log_oom(); + if (!options) { + log_oom(); + goto cleanup; + } } - if (create_disk(name, device, arg_proc_cmdline_keyfile, options) < 0) - r = EXIT_FAILURE; + if (create_disk(name, device, arg_keyfile, options) < 0) + goto cleanup; } - return r; + r2 = EXIT_SUCCESS; + +cleanup: + strv_free(arg_disks); + strv_free(arg_options); + free(arg_keyfile); + + return r != EXIT_SUCCESS ? r : r2; } diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index 8facc88bb..4a5f6b17c 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -72,38 +72,24 @@ static void start_target(const char *target) { log_error("Failed to start unit: %s", bus_error_message(&error, -r)); } -static int parse_proc_cmdline(void) { - _cleanup_free_ char *line = NULL; - char *w, *state; - size_t l; - int r; - - r = proc_cmdline(&line); - if (r < 0) - log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); - if (r <= 0) - return 0; - - FOREACH_WORD_QUOTED(w, l, line, state) { - - if (strneq(w, "fsck.mode=auto", l)) - arg_force = arg_skip = false; - else if (strneq(w, "fsck.mode=force", l)) - arg_force = true; - else if (strneq(w, "fsck.mode=skip", l)) - arg_skip = true; - else if (startswith(w, "fsck")) - log_warning("Invalid fsck parameter. Ignoring."); +static int parse_proc_cmdline_word(const char *w) { + if (streq(w, "fsck.mode=auto")) + arg_force = arg_skip = false; + else if (streq(w, "fsck.mode=force")) + arg_force = true; + else if (streq(w, "fsck.mode=skip")) + arg_skip = true; + else if (startswith(w, "fsck")) + log_warning("Invalid fsck parameter. Ignoring."); #ifdef HAVE_SYSV_COMPAT - else if (strneq(w, "fastboot", l)) { - log_error("Please pass 'fsck.mode=skip' rather than 'fastboot' on the kernel command line."); - arg_skip = true; - } else if (strneq(w, "forcefsck", l)) { - log_error("Please pass 'fsck.mode=force' rather than 'forcefsck' on the kernel command line."); - arg_force = true; - } -#endif + else if (streq(w, "fastboot")) { + log_error("Please pass 'fsck.mode=skip' rather than 'fastboot' on the kernel command line."); + arg_skip = true; + } else if (streq(w, "forcefsck")) { + log_error("Please pass 'fsck.mode=force' rather than 'forcefsck' on the kernel command line."); + arg_force = true; } +#endif return 0; } @@ -229,7 +215,7 @@ int main(int argc, char *argv[]) { umask(0022); - parse_proc_cmdline(); + parse_proc_cmdline(parse_proc_cmdline_word); test_files(); if (!arg_force && arg_skip) diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 0336888b0..5521a864d 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -501,47 +501,30 @@ static int parse_new_root_from_proc_cmdline(void) { return (r < 0) ? r : 0; } -static int parse_proc_cmdline(void) { - _cleanup_free_ char *line = NULL; - char *w, *state; - size_t l; +static int parse_proc_cmdline_word(const char *word) { int r; - r = proc_cmdline(&line); - if (r < 0) - log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); - if (r <= 0) - return 0; - - FOREACH_WORD_QUOTED(w, l, line, state) { - _cleanup_free_ char *word = NULL; + if (startswith(word, "fstab=")) { + r = parse_boolean(word + 6); + if (r < 0) + log_warning("Failed to parse fstab switch %s. Ignoring.", word + 6); + else + arg_enabled = r; - word = strndup(w, l); - if (!word) - return log_oom(); + } else if (startswith(word, "rd.fstab=")) { - if (startswith(word, "fstab=")) { - r = parse_boolean(word + 6); + if (in_initrd()) { + r = parse_boolean(word + 9); if (r < 0) - log_warning("Failed to parse fstab switch %s. Ignoring.", word + 6); + log_warning("Failed to parse fstab switch %s. Ignoring.", word + 9); else arg_enabled = r; + } - } else if (startswith(word, "rd.fstab=")) { - - if (in_initrd()) { - r = parse_boolean(word + 9); - if (r < 0) - log_warning("Failed to parse fstab switch %s. Ignoring.", word + 9); - else - arg_enabled = r; - } - - } else if (startswith(word, "fstab.") || - (in_initrd() && startswith(word, "rd.fstab."))) { + } else if (startswith(word, "fstab.") || + (in_initrd() && startswith(word, "rd.fstab."))) { - log_warning("Unknown kernel switch %s. Ignoring.", word); - } + log_warning("Unknown kernel switch %s. Ignoring.", word); } return 0; @@ -564,7 +547,7 @@ int main(int argc, char *argv[]) { umask(0022); - if (parse_proc_cmdline() < 0) + if (parse_proc_cmdline(parse_proc_cmdline_word) < 0) return EXIT_FAILURE; if (in_initrd()) diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index 3ac25fa98..1a32d26b2 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -69,39 +69,19 @@ static int add_modules(const char *p) { return 0; } -static int parse_proc_cmdline(void) { - _cleanup_free_ char *line = NULL; - char *w, *state; - size_t l; +static int parse_proc_cmdline_word(const char *word) { int r; - r = proc_cmdline(&line); - if (r < 0) - log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); - if (r <= 0) - return 0; - - FOREACH_WORD_QUOTED(w, l, line, state) { - _cleanup_free_ char *word; - - word = strndup(w, l); - if (!word) - return log_oom(); - - if (startswith(word, "modules-load=")) { + if (startswith(word, "modules-load=")) { + r = add_modules(word + 13); + if (r < 0) + return r; - r = add_modules(word + 13); + } else if (startswith(word, "rd.modules-load=")) { + if (in_initrd()) { + r = add_modules(word + 16); if (r < 0) return r; - - } else if (startswith(word, "rd.modules-load=")) { - - if (in_initrd()) { - r = add_modules(word + 16); - if (r < 0) - return r; - } - } } @@ -273,7 +253,7 @@ int main(int argc, char *argv[]) { umask(0022); - if (parse_proc_cmdline() < 0) + if (parse_proc_cmdline(parse_proc_cmdline_word) < 0) return EXIT_FAILURE; ctx = kmod_new(NULL, NULL); diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c index 622952a6e..8ff0f7f0c 100644 --- a/src/quotacheck/quotacheck.c +++ b/src/quotacheck/quotacheck.c @@ -31,35 +31,22 @@ static bool arg_skip = false; static bool arg_force = false; -static int parse_proc_cmdline(void) { - _cleanup_free_ char *line = NULL; - char *w, *state; - size_t l; - int r; - - r = proc_cmdline(&line); - if (r < 0) - log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); - if (r <= 0) - return 0; - - FOREACH_WORD_QUOTED(w, l, line, state) { - - if (strneq(w, "quotacheck.mode=auto", l)) - arg_force = arg_skip = false; - else if (strneq(w, "quotacheck.mode=force", l)) - arg_force = true; - else if (strneq(w, "quotacheck.mode=skip", l)) - arg_skip = true; - else if (startswith(w, "quotacheck")) - log_warning("Invalid quotacheck parameter. Ignoring."); +static int parse_proc_cmdline_word(const char *w) { + if (streq(w, "quotacheck.mode=auto")) + arg_force = arg_skip = false; + else if (streq(w, "quotacheck.mode=force")) + arg_force = true; + else if (streq(w, "quotacheck.mode=skip")) + arg_skip = true; + else if (startswith(w, "quotacheck")) + log_warning("Invalid quotacheck parameter. Ignoring."); #ifdef HAVE_SYSV_COMPAT - else if (strneq(w, "forcequotacheck", l)) { - log_error("Please use 'quotacheck.mode=force' rather than 'forcequotacheck' on the kernel command line."); - arg_force = true; - } -#endif + else if (streq(w, "forcequotacheck")) { + log_error("Please use 'quotacheck.mode=force' rather than 'forcequotacheck' on the kernel command line."); + arg_force = true; } +#endif + return 0; } @@ -93,7 +80,7 @@ int main(int argc, char *argv[]) { umask(0022); - parse_proc_cmdline(); + parse_proc_cmdline(parse_proc_cmdline_word); test_files(); if (!arg_force) { diff --git a/src/shared/util.c b/src/shared/util.c index b1a9db1d4..d95a4b4ab 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -5943,6 +5943,35 @@ int proc_cmdline(char **ret) { return 1; } +int parse_proc_cmdline(int (*parse_word)(const char *word)) { + _cleanup_free_ char *line = NULL; + char *w, *state; + size_t l; + int r; + + r = proc_cmdline(&line); + if (r < 0) + log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r)); + if (r <= 0) + return 0; + + FOREACH_WORD_QUOTED(w, l, line, state) { + _cleanup_free_ char *word; + + word = strndup(w, l); + if (!word) + return log_oom(); + + r = parse_word(word); + if (r < 0) { + log_error("Failed on cmdline argument %s: %s", word, strerror(-r)); + return r; + } + } + + return 0; +} + int container_get_leader(const char *machine, pid_t *pid) { _cleanup_free_ char *s = NULL, *class = NULL; const char *p; diff --git a/src/shared/util.h b/src/shared/util.h index fcb45b5f5..4bed5b484 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -852,6 +852,7 @@ static inline void qsort_safe(void *base, size_t nmemb, size_t size, } int proc_cmdline(char **ret); +int parse_proc_cmdline(int (*parse_word)(const char *word)); int container_get_leader(const char *machine, pid_t *pid); -- 2.30.2