diff --git a/src/basic/fileio.c b/src/basic/fileio.c index f19326b7110..001c19378e5 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1327,33 +1327,31 @@ int read_timestamp_file(const char *fn, usec_t *ret) { return 0; } -int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space) { - int r; - +int fputs_with_separator(FILE *f, const char *s, const char *separator, bool *space) { assert(s); + assert(space); - /* Outputs the specified string with fputs(), but optionally prefixes it with a separator. The *space parameter - * when specified shall initially point to a boolean variable initialized to false. It is set to true after the - * first invocation. This call is supposed to be use in loops, where a separator shall be inserted between each - * element, but not before the first one. */ + /* Outputs the specified string with fputs(), but optionally prefixes it with a separator. + * The *space parameter when specified shall initially point to a boolean variable initialized + * to false. It is set to true after the first invocation. This call is supposed to be use in loops, + * where a separator shall be inserted between each element, but not before the first one. */ if (!f) f = stdout; - if (space) { - if (!separator) - separator = " "; + if (!separator) + separator = " "; - if (*space) { - r = fputs(separator, f); - if (r < 0) - return r; - } + if (*space) + if (fputs(separator, f) < 0) + return -EIO; - *space = true; - } + *space = true; - return fputs(s, f); + if (fputs(s, f) < 0) + return -EIO; + + return 0; } /* A bitmask of the EOL markers we know */ diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 5b247bc1011..03c3f3ff283 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -143,7 +143,7 @@ int fflush_sync_and_check(FILE *f); int write_timestamp_file_atomic(const char *fn, usec_t n); int read_timestamp_file(const char *fn, usec_t *ret); -int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space); +int fputs_with_separator(FILE *f, const char *s, const char *separator, bool *space); typedef enum ReadLineFlags { READ_LINE_ONLY_NUL = 1 << 0, diff --git a/src/basic/ordered-set.c b/src/basic/ordered-set.c index b4c2588395d..65cf3a026f4 100644 --- a/src/basic/ordered-set.c +++ b/src/basic/ordered-set.c @@ -91,13 +91,16 @@ void ordered_set_print(FILE *f, const char *field, OrderedSet *s) { bool space = false; char *p; + assert(f); + assert(field); + if (ordered_set_isempty(s)) return; fputs(field, f); ORDERED_SET_FOREACH(p, s) - fputs_with_space(f, p, NULL, &space); + fputs_with_separator(f, p, NULL, &space); fputc('\n', f); } diff --git a/src/basic/strv.c b/src/basic/strv.c index e32653818b0..72cbbfe2f4e 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -894,13 +894,15 @@ int fputstrv(FILE *f, char * const *l, const char *separator, bool *space) { bool b = false; int r; + assert(f); + /* Like fputs(), but for strv, and with a less stupid argument order */ if (!space) space = &b; STRV_FOREACH(s, l) { - r = fputs_with_space(f, *s, separator, space); + r = fputs_with_separator(f, *s, separator, space); if (r < 0) return r; } diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 3c0b597470a..3a1d2f38dbb 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -191,6 +191,8 @@ static int mount_array_add_swap(bool for_initrd, const char *str) { static int write_options(FILE *f, const char *options) { _cleanup_free_ char *o = NULL; + assert(f); + if (isempty(options)) return 0; @@ -208,6 +210,9 @@ static int write_options(FILE *f, const char *options) { static int write_what(FILE *f, const char *what) { _cleanup_free_ char *w = NULL; + assert(f); + assert(what); + w = specifier_escape(what); if (!w) return log_oom(); @@ -324,25 +329,30 @@ static int write_timeout( const char *where, const char *opts, const char *filter, - const char *variable) { + const char *unit_setting) { _cleanup_free_ char *timeout = NULL; usec_t u; int r; + assert(f); + assert(where); + assert(filter); + assert(unit_setting); + r = fstab_filter_options(opts, filter, NULL, &timeout, NULL, NULL); if (r < 0) - return log_warning_errno(r, "Failed to parse options: %m"); + return log_error_errno(r, "Failed to parse options for '%s': %m", where); if (r == 0) return 0; r = parse_sec_fix_0(timeout, &u); if (r < 0) { - log_warning("Failed to parse timeout for %s, ignoring: %s", where, timeout); + log_warning_errno(r, "Failed to parse timeout '%s' for '%s', ignoring: %m", timeout, where); return 0; } - fprintf(f, "%s=%s\n", variable, FORMAT_TIMESPAN(u, 0)); + fprintf(f, "%s=%s\n", unit_setting, FORMAT_TIMESPAN(u, 0)); return 0; } @@ -359,114 +369,118 @@ static int write_mount_timeout(FILE *f, const char *where, const char *opts) { static int write_dependency( FILE *f, + const char *where, const char *opts, const char *filter, - const char *format) { + const char* const *unit_settings) { - _cleanup_strv_free_ char **names = NULL, **units = NULL; - _cleanup_free_ char *res = NULL; + _cleanup_strv_free_ char **unit_names = NULL; + _cleanup_free_ char *units = NULL; int r; assert(f); - assert(opts); + assert(filter); + assert(unit_settings); - r = fstab_filter_options(opts, filter, NULL, NULL, &names, NULL); + r = fstab_filter_options(opts, filter, NULL, NULL, &unit_names, NULL); if (r < 0) - return log_warning_errno(r, "Failed to parse options: %m"); + return log_error_errno(r, "Failed to parse options for '%s': %m", where); if (r == 0) return 0; - STRV_FOREACH(s, names) { - char *x; + STRV_FOREACH(s, unit_names) { + _cleanup_free_ char *mangled = NULL; - r = unit_name_mangle_with_suffix(*s, "as dependency", 0, ".mount", &x); + r = unit_name_mangle_with_suffix(*s, "as dependency", 0, ".mount", &mangled); if (r < 0) - return log_error_errno(r, "Failed to generate unit name: %m"); + return log_error_errno(r, "Failed to generate dependency unit name for '%s': %m", where); - r = strv_consume(&units, x); - if (r < 0) + if (!strextend_with_separator(&units, " ", mangled)) return log_oom(); } - if (units) { - res = strv_join(units, " "); - if (!res) - return log_oom(); - - DISABLE_WARNING_FORMAT_NONLITERAL; - fprintf(f, format, res); - REENABLE_WARNING; - } + STRV_FOREACH(setting, unit_settings) + fprintf(f, "%s=%s\n", *setting, units); return 0; } -static int write_after(FILE *f, const char *opts) { - return write_dependency(f, opts, - "x-systemd.after\0", "After=%1$s\n"); +static int write_after(FILE *f, const char *where, const char *opts) { + return write_dependency(f, where, opts, + "x-systemd.after\0", STRV_MAKE_CONST("After")); } -static int write_requires_after(FILE *f, const char *opts) { - return write_dependency(f, opts, - "x-systemd.requires\0", "After=%1$s\nRequires=%1$s\n"); +static int write_requires_after(FILE *f, const char *where, const char *opts) { + return write_dependency(f, where, opts, + "x-systemd.requires\0", STRV_MAKE_CONST("Requires", "After")); } -static int write_before(FILE *f, const char *opts) { - return write_dependency(f, opts, - "x-systemd.before\0", "Before=%1$s\n"); +static int write_before(FILE *f, const char *where, const char *opts) { + return write_dependency(f, where, opts, + "x-systemd.before\0", STRV_MAKE_CONST("Before")); } -static int write_mounts_for(const char *x_opt, const char *unit_setting, FILE *f, const char *opts) { +static int write_mounts_for( + FILE *f, + const char *where, + const char *opts, + const char *filter, + const char *unit_setting) { + _cleanup_strv_free_ char **paths = NULL, **paths_escaped = NULL; - _cleanup_free_ char *res = NULL; int r; - assert(x_opt); - assert(unit_setting); assert(f); - assert(opts); + assert(where); + assert(filter); + assert(unit_setting); - r = fstab_filter_options(opts, x_opt, NULL, NULL, &paths, NULL); + r = fstab_filter_options(opts, filter, NULL, NULL, &paths, NULL); if (r < 0) - return log_warning_errno(r, "Failed to parse options: %m"); + return log_error_errno(r, "Failed to parse options for '%s': %m", where); if (r == 0) return 0; r = specifier_escape_strv(paths, &paths_escaped); if (r < 0) - return log_error_errno(r, "Failed to escape paths: %m"); + return log_error_errno(r, "Failed to escape paths for '%s': %m", where); - res = strv_join(paths_escaped, " "); - if (!res) - return log_oom(); - - fprintf(f, "%s=%s\n", unit_setting, res); + fprintf(f, "%s=", unit_setting); + fputstrv(f, paths_escaped, NULL, NULL); + fputc('\n', f); return 0; } -static int write_extra_dependencies(FILE *f, const char *opts) { +static int write_extra_dependencies(FILE *f, const char *where, const char *opts) { int r; assert(f); - if (opts) { - r = write_after(f, opts); - if (r < 0) - return r; - r = write_requires_after(f, opts); - if (r < 0) - return r; - r = write_before(f, opts); - if (r < 0) - return r; - r = write_mounts_for("x-systemd.requires-mounts-for\0", "RequiresMountsFor", f, opts); - if (r < 0) - return r; - r = write_mounts_for("x-systemd.wants-mounts-for\0", "WantsMountsFor", f, opts); - if (r < 0) - return r; - } + if (isempty(opts)) + return 0; + + r = write_after(f, where, opts); + if (r < 0) + return r; + + r = write_requires_after(f, where, opts); + if (r < 0) + return r; + + r = write_before(f, where, opts); + if (r < 0) + return r; + + r = write_mounts_for(f, where, opts, + "x-systemd.requires-mounts-for\0", "RequiresMountsFor"); + if (r < 0) + return r; + + r = write_mounts_for(f, where, opts, + "x-systemd.wants-mounts-for\0", "WantsMountsFor"); + if (r < 0) + return r; return 0; } @@ -481,17 +495,13 @@ static int mandatory_mount_drop_unapplicable_options( assert(flags); assert(where); - assert(options); assert(ret_options); if (!(*flags & (MOUNT_NOAUTO|MOUNT_NOFAIL|MOUNT_AUTOMOUNT))) { - _cleanup_free_ char *opts = NULL; + r = strdup_or_null(options, ret_options); + if (r < 0) + return r; - opts = strdup(options); - if (!opts) - return -ENOMEM; - - *ret_options = TAKE_PTR(opts); return 0; } @@ -527,7 +537,6 @@ static int add_mount( assert(what); assert(where); - assert(opts); assert(target_unit); assert(source); @@ -556,16 +565,16 @@ static int add_mount( if (r < 0) return r; - if (path_equal(where, "/")) { + if (PATH_IN_SET(where, "/", "/usr")) { r = mandatory_mount_drop_unapplicable_options(&flags, where, opts, &opts_root_filtered); if (r < 0) return r; opts = opts_root_filtered; if (!strv_isempty(wanted_by)) - log_debug("Ignoring 'x-systemd.wanted-by=' option for root device."); + log_debug("Ignoring 'x-systemd.wanted-by=' option for root/usr device."); if (!strv_isempty(required_by)) - log_debug("Ignoring 'x-systemd.required-by=' option for root device."); + log_debug("Ignoring 'x-systemd.required-by=' option for root/usr device."); required_by = strv_free(required_by); wanted_by = strv_free(wanted_by); @@ -611,7 +620,7 @@ static int add_mount( f); } - r = write_extra_dependencies(f, opts); + r = write_extra_dependencies(f, where, opts); if (r < 0) return r; @@ -840,6 +849,9 @@ static int add_sysusr_sysroot_usr_bind_mount(const char *source) { static MountPointFlags fstab_options_to_flags(const char *options, bool is_swap) { MountPointFlags flags = 0; + if (isempty(options)) + return 0; + if (fstab_test_option(options, "x-systemd.makefs\0")) flags |= MOUNT_MAKEFS; if (fstab_test_option(options, "x-systemd.growfs\0")) @@ -915,7 +927,6 @@ static int parse_fstab_one( assert(what_original); assert(fstype); - assert(options); if (prefix_sysroot && !mount_in_initrd(where_original, options, accept_root)) return 0; @@ -1587,7 +1598,7 @@ static int determine_usr(void) { * with /sysroot/etc/fstab available, and then we can write additional units based * on that file. */ static int run_generator(void) { - int r = 0; + int r; r = proc_cmdline_parse(parse_proc_cmdline_item, NULL, 0); if (r < 0) diff --git a/src/network/networkd-state-file.c b/src/network/networkd-state-file.c index b79b898832b..5b2e8a2de4f 100644 --- a/src/network/networkd-state-file.c +++ b/src/network/networkd-state-file.c @@ -537,7 +537,7 @@ static void link_save_domains(Link *link, FILE *f, OrderedSet *static_domains, D assert(f); ORDERED_SET_FOREACH(p, static_domains) - fputs_with_space(f, p, NULL, &space); + fputs_with_separator(f, p, NULL, &space); if (use_domains == DHCP_USE_DOMAINS_NO) return; @@ -547,7 +547,7 @@ static void link_save_domains(Link *link, FILE *f, OrderedSet *static_domains, D char **domains; if (sd_dhcp_lease_get_domainname(link->dhcp_lease, &domainname) >= 0) - fputs_with_space(f, domainname, NULL, &space); + fputs_with_separator(f, domainname, NULL, &space); if (sd_dhcp_lease_get_search_domains(link->dhcp_lease, &domains) >= 0) fputstrv(f, domains, NULL, &space); } @@ -563,7 +563,7 @@ static void link_save_domains(Link *link, FILE *f, OrderedSet *static_domains, D NDiscDNSSL *dd; SET_FOREACH(dd, link->ndisc_dnssl) - fputs_with_space(f, NDISC_DNSSL_DOMAIN(dd), NULL, &space); + fputs_with_separator(f, NDISC_DNSSL_DOMAIN(dd), NULL, &space); } } @@ -652,7 +652,7 @@ static int link_save(Link *link) { if (!escaped) return -ENOMEM; - fputs_with_space(f, escaped, ":", &space); + fputs_with_separator(f, escaped, ":", &space); } fputs("\"\n", f); @@ -782,7 +782,7 @@ static int link_save(Link *link) { fputs("DNSSEC_NTA=", f); space = false; SET_FOREACH(n, nta_anchors) - fputs_with_space(f, n, NULL, &space); + fputs_with_separator(f, n, NULL, &space); fputc('\n', f); } } diff --git a/src/shared/fstab-util.c b/src/shared/fstab-util.c index bfb0f05c4fe..efefd9a5259 100644 --- a/src/shared/fstab-util.c +++ b/src/shared/fstab-util.c @@ -148,16 +148,16 @@ int fstab_filter_options( char ***ret_values, char **ret_filtered) { - const char *namefound = NULL, *x; - _cleanup_strv_free_ char **stor = NULL, **values = NULL; - _cleanup_free_ char *value = NULL, **filtered = NULL; + _cleanup_strv_free_ char **values = NULL; + _cleanup_free_ char *value = NULL, *filtered = NULL; + const char *namefound = NULL; int r; - assert(names && *names); + assert(!isempty(names)); assert(!(ret_value && ret_values)); if (!opts) - goto answer; + goto finish; /* Finds any options matching 'names', and returns: * - the last matching option name in ret_namefound, @@ -169,50 +169,49 @@ int fstab_filter_options( * * Returns negative on error, true if any matching options were found, false otherwise. */ - if (ret_filtered || ret_value || ret_values) { + if (ret_value || ret_values || ret_filtered) { + _cleanup_strv_free_ char **opts_split = NULL; + _cleanup_free_ char **filtered_strv = NULL; /* strings are owned by 'opts_split' */ + /* For backwards compatibility, we need to pass-through escape characters. * The only ones we "consume" are the ones used as "\," or "\\". */ - r = strv_split_full(&stor, opts, ",", EXTRACT_UNESCAPE_SEPARATORS | EXTRACT_UNESCAPE_RELAX); + r = strv_split_full(&opts_split, opts, ",", EXTRACT_UNESCAPE_SEPARATORS|EXTRACT_UNESCAPE_RELAX); if (r < 0) return r; - filtered = memdup(stor, sizeof(char*) * (strv_length(stor) + 1)); - if (!filtered) - return -ENOMEM; + STRV_FOREACH(opt, opts_split) { + bool found = false; + const char *x; - char **t = filtered; - for (char **s = t; *s; s++) { NULSTR_FOREACH(name, names) { - x = startswith(*s, name); + x = startswith(*opt, name); if (!x) continue; - /* Match name, but when ret_values, only when followed by assignment. */ + + /* If ret_values, only accept settings followed by assignment. */ if (*x == '=' || (!ret_values && *x == '\0')) { - /* Keep the last occurrence found */ namefound = name; - goto found; + found = true; + break; } } - *t = *s; - t++; - continue; - found: - if (ret_value || ret_values) { - assert(IN_SET(*x, '=', '\0')); - - if (ret_value) { + if (found) { + if (ret_value) r = free_and_strdup(&value, *x == '=' ? x + 1 : NULL); - if (r < 0) - return r; - } else if (*x) { + else if (ret_values) r = strv_extend(&values, x + 1); - if (r < 0) - return r; - } - } + else + r = 0; + } else + r = strv_push(&filtered_strv, *opt); + if (r < 0) + return r; } - *t = NULL; + + filtered = strv_join_full(filtered_strv, ",", NULL, /* escape_separator = */ true); + if (!filtered) + return -ENOMEM; } else for (const char *word = opts;;) { const char *end = word; @@ -232,7 +231,7 @@ int fstab_filter_options( } NULSTR_FOREACH(name, names) { - x = startswith(word, name); + const char *x = startswith(word, name); if (!x || x > end) continue; @@ -249,39 +248,32 @@ int fstab_filter_options( word = end + 1; } -answer: +finish: if (ret_namefound) - *ret_namefound = namefound; - if (ret_filtered) { - char *f; - - f = strv_join_full(filtered, ",", NULL, true); - if (!f) - return -ENOMEM; - - *ret_filtered = f; - } + *ret_namefound = namefound; /* owned by 'names' (passed-in) */ if (ret_value) *ret_value = TAKE_PTR(value); if (ret_values) *ret_values = TAKE_PTR(values); + if (ret_filtered) + *ret_filtered = TAKE_PTR(filtered); return !!namefound; } -int fstab_find_pri(const char *options, int *ret) { - _cleanup_free_ char *opt = NULL; +int fstab_find_pri(const char *opts, int *ret) { + _cleanup_free_ char *v = NULL; int r, pri; assert(ret); - r = fstab_filter_options(options, "pri\0", NULL, &opt, NULL, NULL); + r = fstab_filter_options(opts, "pri\0", NULL, &v, NULL, NULL); if (r < 0) return r; - if (r == 0 || !opt) + if (r == 0 || !v) return 0; - r = safe_atoi(opt, &pri); + r = safe_atoi(v, &pri); if (r < 0) return r; diff --git a/src/shared/fstab-util.h b/src/shared/fstab-util.h index 9cf34f0f8ba..c01b0b93d13 100644 --- a/src/shared/fstab-util.h +++ b/src/shared/fstab-util.h @@ -32,23 +32,20 @@ int fstab_filter_options( char **ret_value, char ***ret_values, char **ret_filtered); - static inline bool fstab_test_option(const char *opts, const char *names) { - return !!fstab_filter_options(opts, names, NULL, NULL, NULL, NULL); + return fstab_filter_options(opts, names, NULL, NULL, NULL, NULL); } - -int fstab_find_pri(const char *options, int *ret); - static inline bool fstab_test_yes_no_option(const char *opts, const char *yes_no) { - const char *opt; + const char *opt_found; /* If first name given is last, return 1. * If second name given is last or neither is found, return 0. */ - assert_se(fstab_filter_options(opts, yes_no, &opt, NULL, NULL, NULL) >= 0); + assert_se(fstab_filter_options(opts, yes_no, &opt_found, NULL, NULL, NULL) >= 0); - return opt == yes_no; + return opt_found == yes_no; } +int fstab_find_pri(const char *opts, int *ret); char *fstab_node_to_udev_node(const char *p); diff --git a/src/test/test-fstab-util.c b/src/test/test-fstab-util.c index 89365b0bc5a..49561a21d70 100644 --- a/src/test/test-fstab-util.c +++ b/src/test/test-fstab-util.c @@ -116,7 +116,7 @@ TEST(fstab_filter_options) { do_fstab_filter_options(" opt ", "opt\0x-opt\0", 0, 0, NULL, NULL, "", NULL); /* check function with NULL args */ - do_fstab_filter_options(NULL, "opt\0", 0, 0, NULL, NULL, "", ""); + do_fstab_filter_options(NULL, "opt\0", 0, 0, NULL, NULL, "", NULL); do_fstab_filter_options("", "opt\0", 0, 0, NULL, NULL, "", ""); /* unnecessary comma separators */ diff --git a/test/units/testsuite-81.fstab-generator.sh b/test/units/testsuite-81.fstab-generator.sh index 50c4b2f47d4..4a69245ed87 100755 --- a/test/units/testsuite-81.fstab-generator.sh +++ b/test/units/testsuite-81.fstab-generator.sh @@ -148,7 +148,7 @@ check_fstab_mount_units() { fi if [[ -n "$opts" ]] && [[ "$opts" != defaults ]]; then # Some options are not propagated to the generated unit - if [[ "$where" == / ]]; then + if [[ "$where" == / || "$where" == /usr ]]; then filtered_options="$(opt_filter "$opts" "(noauto|nofail|x-systemd.(wanted-by=|required-by=|automount|device-timeout=))")" else filtered_options="$(opt_filter "$opts" "^x-systemd.device-timeout=")"