From: Zbigniew Jędrzejewski-Szmek Date: Tue, 16 Feb 2021 15:04:41 +0000 (+0100) Subject: Refactor strv_env_replace() into strv_env_replace_consume() X-Git-Tag: v248-rc1~92^2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F18601%2Fhead;p=thirdparty%2Fsystemd.git Refactor strv_env_replace() into strv_env_replace_consume() All callers of strv_env_replace() would free the argument on error. So let's follow the same pattern as with strv_consume (and similar naming) and unconditionally "use up" the argument. --- diff --git a/src/basic/env-file.c b/src/basic/env-file.c index 99c3e3f4a31..11362d8556b 100644 --- a/src/basic/env-file.c +++ b/src/basic/env-file.c @@ -385,11 +385,9 @@ static int load_env_file_push( if (!p) return -ENOMEM; - r = strv_env_replace(m, p); - if (r < 0) { - free(p); + r = strv_env_replace_consume(m, p); + if (r < 0) return r; - } if (n_pushed) (*n_pushed)++; diff --git a/src/basic/env-util.c b/src/basic/env-util.c index b1313dbfff8..25c4b7c5a41 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -363,7 +363,7 @@ char **strv_env_unset_many(char **l, ...) { return l; } -int strv_env_replace(char ***l, char *p) { +int strv_env_replace_consume(char ***l, char *p) { const char *t, *name; char **f; int r; @@ -371,11 +371,15 @@ int strv_env_replace(char ***l, char *p) { assert(p); /* Replace first occurrence of the env var or add a new one in the string list. Drop other - * occurrences. Edits in-place. Does not copy p. p must be a valid key=value assignment. */ + * occurrences. Edits in-place. Does not copy p and CONSUMES p EVEN ON FAILURE. + * + * p must be a valid key=value assignment. */ t = strchr(p, '='); - if (!t) + if (!t) { + free(p); return -EINVAL; + } name = strndupa(p, t - p); @@ -387,7 +391,7 @@ int strv_env_replace(char ***l, char *p) { } /* We didn't find a match, we need to append p or create a new strv */ - r = strv_push(l, p); + r = strv_consume(l, p); if (r < 0) return r; @@ -395,24 +399,16 @@ int strv_env_replace(char ***l, char *p) { } int strv_env_replace_strdup(char ***l, const char *assignment) { - int r; + /* Like strv_env_replace_consume(), but copies the argument. */ - /* Like strv_env_replace(), but copies the argument. */ - - _cleanup_free_ char *p = strdup(assignment); + char *p = strdup(assignment); if (!p) return -ENOMEM; - r = strv_env_replace(l, p); - if (r < 0) - return r; - TAKE_PTR(p); - return r; + return strv_env_replace_consume(l, p); } int strv_env_assign(char ***l, const char *key, const char *value) { - int r; - if (!env_name_is_valid(key)) return -EINVAL; @@ -427,11 +423,7 @@ int strv_env_assign(char ***l, const char *key, const char *value) { if (!p) return -ENOMEM; - r = strv_env_replace(l, p); - if (r < 0) - return r; - TAKE_PTR(p); - return r; + return strv_env_replace_consume(l, p); } char *strv_env_get_n(char **l, const char *name, size_t k, unsigned flags) { diff --git a/src/basic/env-util.h b/src/basic/env-util.h index c7b72b4b471..347ea33e666 100644 --- a/src/basic/env-util.h +++ b/src/basic/env-util.h @@ -44,7 +44,7 @@ char **strv_env_delete(char **x, size_t n_lists, ...); /* New copy */ char **strv_env_unset(char **l, const char *p); /* In place ... */ char **strv_env_unset_many(char **l, ...) _sentinel_; -int strv_env_replace(char ***l, char *p); /* In place ... */ +int strv_env_replace_consume(char ***l, char *p); /* In place ... */ int strv_env_replace_strdup(char ***l, const char *assignment); int strv_env_assign(char ***l, const char *key, const char *value); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index d0aeaf1c76b..dfab38f77bd 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2636,7 +2636,7 @@ int config_parse_environ( } for (const char *p = rvalue;; ) { - _cleanup_free_ char *word = NULL, *k = NULL; + _cleanup_free_ char *word = NULL, *resolved = NULL; r = extract_first_word(&p, &word, NULL, EXTRACT_CUNESCAPE|EXTRACT_UNQUOTE); if (r == 0) @@ -2650,26 +2650,24 @@ int config_parse_environ( } if (u) { - r = unit_full_printf(u, word, &k); + r = unit_full_printf(u, word, &resolved); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to resolve unit specifiers in %s, ignoring: %m", word); continue; } } else - k = TAKE_PTR(word); + resolved = TAKE_PTR(word); - if (!env_assignment_is_valid(k)) { + if (!env_assignment_is_valid(resolved)) { log_syntax(unit, LOG_WARNING, filename, line, 0, - "Invalid environment assignment, ignoring: %s", k); + "Invalid environment assignment, ignoring: %s", resolved); continue; } - r = strv_env_replace(env, k); + r = strv_env_replace_consume(env, TAKE_PTR(resolved)); if (r < 0) - return log_oom(); - - k = NULL; + return log_error_errno(r, "Failed to update environment: %m"); } } diff --git a/src/shared/serialize.c b/src/shared/serialize.c index 45f57d6a531..42fe5db3faf 100644 --- a/src/shared/serialize.c +++ b/src/shared/serialize.c @@ -175,7 +175,7 @@ int deserialize_dual_timestamp(const char *value, dual_timestamp *t) { } int deserialize_environment(const char *value, char ***list) { - _cleanup_free_ char *unescaped = NULL; + char *unescaped; int r; assert(value); @@ -187,11 +187,9 @@ int deserialize_environment(const char *value, char ***list) { if (r < 0) return log_error_errno(r, "Failed to unescape: %m"); - r = strv_env_replace(list, unescaped); + r = strv_env_replace_consume(list, unescaped); if (r < 0) return log_error_errno(r, "Failed to append environment variable: %m"); - - unescaped = NULL; /* now part of 'list' */ return 0; }