From: Mike Yuan Date: Sat, 20 Jan 2024 14:16:52 +0000 (+0800) Subject: fstab-util: clean up fstab_filter_options X-Git-Tag: v256-rc1~1033^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5d19388349415d9835c7a787a0bd18543adf8a0b;p=thirdparty%2Fsystemd.git fstab-util: clean up fstab_filter_options Let's get rid of the confusing goto so that the flow is more straightforward. Note that the behavior is slightly changed: previously, ret_filtered would be an empty string even if the original opts passed in is NULL, but after this commit it returns NULL too. But this shouldn't matter, as all our code handles NULL opts gracefully. --- 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 */