Skip to content

Commit

Permalink
Merge pull request systemd#27424 from dtardon/auto-cleanup
Browse files Browse the repository at this point in the history 
More automatic cleanup
  • Loading branch information
@yuwata
yuwata authored Apr 28, 2023
2 parents c63dde8 + 360179e commit 75fd8ad
Show file tree
Hide file tree
Showing 15 changed files with 106 additions and 112 deletions.
73 changes: 33 additions & 40 deletions src/basic/env-util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,20 +136,20 @@ bool strv_env_name_or_assignment_is_valid(char **l) {
return true;
}

static int env_append(char **r, char ***k, char **a) {
assert(r);
static int env_append(char **e, char ***k, char **a) {
assert(e);
assert(k);
assert(*k >= r);
assert(*k >= e);

if (!a)
return 0;

/* Expects the following arguments: 'r' shall point to the beginning of an strv we are going to append to, 'k'
/* Expects the following arguments: 'e' shall point to the beginning of an strv we are going to append to, 'k'
* to a pointer pointing to the NULL entry at the end of the same array. 'a' shall point to another strv.
*
* This call adds every entry of 'a' to 'r', either overriding an existing matching entry, or appending to it.
* This call adds every entry of 'a' to 'e', either overriding an existing matching entry, or appending to it.
*
* This call assumes 'r' has enough pre-allocated space to grow by all of 'a''s items. */
* This call assumes 'e' has enough pre-allocated space to grow by all of 'a''s items. */

for (; *a; a++) {
char **j, *c;
Expand All @@ -159,7 +159,7 @@ static int env_append(char **r, char ***k, char **a) {
if ((*a)[n] == '=')
n++;

for (j = r; j < *k; j++)
for (j = e; j < *k; j++)
if (strneq(*j, *a, n))
break;

Expand Down Expand Up @@ -266,16 +266,16 @@ static bool env_entry_has_name(const char *entry, const char *name) {

char **strv_env_delete(char **x, size_t n_lists, ...) {
size_t n, i = 0;
char **r;
_cleanup_strv_free_ char **t = NULL;
va_list ap;

/* Deletes every entry from x that is mentioned in the other
* string lists */

n = strv_length(x);

r = new(char*, n+1);
if (!r)
t = new(char*, n+1);
if (!t)
return NULL;

STRV_FOREACH(k, x) {
Expand All @@ -290,11 +290,9 @@ char **strv_env_delete(char **x, size_t n_lists, ...) {
}
va_end(ap);

r[i] = strdup(*k);
if (!r[i]) {
strv_free(r);
t[i] = strdup(*k);
if (!t[i])
return NULL;
}

i++;
continue;
Expand All @@ -303,11 +301,11 @@ char **strv_env_delete(char **x, size_t n_lists, ...) {
va_end(ap);
}

r[i] = NULL;
t[i] = NULL;

assert(i <= n);

return r;
return TAKE_PTR(t);
}

char **strv_env_unset(char **l, const char *p) {
Expand Down Expand Up @@ -588,7 +586,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {

const char *e, *word = format, *test_value = NULL; /* test_value is initialized to appease gcc */
char *k;
_cleanup_free_ char *r = NULL;
_cleanup_free_ char *s = NULL;
size_t i, len = 0; /* len is initialized to appease gcc */
int nest = 0;

Expand All @@ -604,31 +602,31 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {

case CURLY:
if (*e == '{') {
k = strnappend(r, word, e-word-1);
k = strnappend(s, word, e-word-1);
if (!k)
return NULL;

free_and_replace(r, k);
free_and_replace(s, k);

word = e-1;
state = VARIABLE;
nest++;
} else if (*e == '$') {
k = strnappend(r, word, e-word);
k = strnappend(s, word, e-word);
if (!k)
return NULL;

free_and_replace(r, k);
free_and_replace(s, k);

word = e+1;
state = WORD;

} else if (flags & REPLACE_ENV_ALLOW_BRACELESS && strchr(VALID_BASH_ENV_NAME_CHARS, *e)) {
k = strnappend(r, word, e-word-1);
k = strnappend(s, word, e-word-1);
if (!k)
return NULL;

free_and_replace(r, k);
free_and_replace(s, k);

word = e-1;
state = VARIABLE_RAW;
Expand All @@ -643,7 +641,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {

t = strv_env_get_n(env, word+2, e-word-2, flags);

if (!strextend(&r, t))
if (!strextend(&s, t))
return NULL;

word = e+1;
Expand Down Expand Up @@ -696,7 +694,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {
else if (!t && state == DEFAULT_VALUE)
t = v = replace_env_n(test_value, e-test_value, env, flags);

if (!strextend(&r, t))
if (!strextend(&s, t))
return NULL;

word = e+1;
Expand All @@ -712,7 +710,7 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {

t = strv_env_get_n(env, word+1, e-word-1, flags);

if (!strextend(&r, t))
if (!strextend(&s, t))
return NULL;

word = e--;
Expand All @@ -728,13 +726,13 @@ char *replace_env_n(const char *format, size_t n, char **env, unsigned flags) {
assert(flags & REPLACE_ENV_ALLOW_BRACELESS);

t = strv_env_get_n(env, word+1, e-word-1, flags);
return strjoin(r, t);
return strjoin(s, t);
} else
return strnappend(r, word, e-word);
return strnappend(s, word, e-word);
}

char **replace_env_argv(char **argv, char **env) {
char **ret;
_cleanup_strv_free_ char **ret = NULL;
size_t k = 0, l = 0;

l = strv_length(argv);
Expand All @@ -748,7 +746,8 @@ char **replace_env_argv(char **argv, char **env) {
/* If $FOO appears as single word, replace it by the split up variable */
if ((*i)[0] == '$' && !IN_SET((*i)[1], '{', '$')) {
char *e;
char **w, **m = NULL;
char **w;
_cleanup_strv_free_ char **m = NULL;
size_t q;

e = strv_env_get(env, *i+1);
Expand All @@ -758,27 +757,23 @@ char **replace_env_argv(char **argv, char **env) {
r = strv_split_full(&m, e, WHITESPACE, EXTRACT_RELAX|EXTRACT_UNQUOTE);
if (r < 0) {
ret[k] = NULL;
strv_free(ret);
return NULL;
}
} else
m = NULL;
}

q = strv_length(m);
l = l + q - 1;

w = reallocarray(ret, l + 1, sizeof(char *));
if (!w) {
ret[k] = NULL;
strv_free(ret);
strv_free(m);
return NULL;
}

ret = w;
if (m) {
memcpy(ret + k, m, q * sizeof(char*));
free(m);
m = mfree(m);
}

k += q;
Expand All @@ -787,15 +782,13 @@ char **replace_env_argv(char **argv, char **env) {

/* If ${FOO} appears as part of a word, replace it by the variable as-is */
ret[k] = replace_env(*i, env, 0);
if (!ret[k]) {
strv_free(ret);
if (!ret[k])
return NULL;
}
k++;
}

ret[k] = NULL;
return ret;
return TAKE_PTR(ret);
}

int getenv_bool(const char *p) {
Expand Down
8 changes: 3 additions & 5 deletions src/basic/path-util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include "time-util.h"

int path_split_and_make_absolute(const char *p, char ***ret) {
char **l;
_cleanup_strv_free_ char **l = NULL;
int r;

assert(p);
Expand All @@ -33,12 +33,10 @@ int path_split_and_make_absolute(const char *p, char ***ret) {
return -ENOMEM;

r = path_strv_make_absolute_cwd(l);
if (r < 0) {
strv_free(l);
if (r < 0)
return r;
}

*ret = l;
*ret = TAKE_PTR(l);
return r;
}

Expand Down
30 changes: 10 additions & 20 deletions src/core/manager.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2019,7 +2019,7 @@ int manager_add_job(
sd_bus_error *error,
Job **ret) {

Transaction *tr;
_cleanup_(transaction_abort_and_freep) Transaction *tr = NULL;
int r;

assert(m);
Expand Down Expand Up @@ -2048,23 +2048,23 @@ int manager_add_job(
IN_SET(mode, JOB_IGNORE_DEPENDENCIES, JOB_IGNORE_REQUIREMENTS),
mode == JOB_IGNORE_DEPENDENCIES, error);
if (r < 0)
goto tr_abort;
return r;

if (mode == JOB_ISOLATE) {
r = transaction_add_isolate_jobs(tr, m);
if (r < 0)
goto tr_abort;
return r;
}

if (mode == JOB_TRIGGERING) {
r = transaction_add_triggering_jobs(tr, unit);
if (r < 0)
goto tr_abort;
return r;
}

r = transaction_activate(tr, m, mode, affected_jobs, error);
if (r < 0)
goto tr_abort;
return r;

log_unit_debug(unit,
"Enqueued job %s/%s as %u", unit->id,
Expand All @@ -2073,13 +2073,8 @@ int manager_add_job(
if (ret)
*ret = tr->anchor_job;

transaction_free(tr);
tr = transaction_free(tr);
return 0;

tr_abort:
transaction_abort(tr);
transaction_free(tr);
return r;
}

int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs, sd_bus_error *e, Job **ret) {
Expand Down Expand Up @@ -2117,7 +2112,7 @@ int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name,

int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error *e) {
int r;
Transaction *tr;
_cleanup_(transaction_abort_and_freep) Transaction *tr = NULL;

assert(m);
assert(unit);
Expand All @@ -2131,22 +2126,17 @@ int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error
/* We need an anchor job */
r = transaction_add_job_and_dependencies(tr, JOB_NOP, unit, NULL, false, false, true, true, e);
if (r < 0)
goto tr_abort;
return r;

/* Failure in adding individual dependencies is ignored, so this always succeeds. */
transaction_add_propagate_reload_jobs(tr, unit, tr->anchor_job, mode == JOB_IGNORE_DEPENDENCIES, e);

r = transaction_activate(tr, m, mode, NULL, e);
if (r < 0)
goto tr_abort;
return r;

transaction_free(tr);
tr = transaction_free(tr);
return 0;

tr_abort:
transaction_abort(tr);
transaction_free(tr);
return r;
}

Job *manager_get_job(Manager *m, uint32_t id) {
Expand Down
19 changes: 16 additions & 3 deletions src/core/transaction.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ static void transaction_delete_unit(Transaction *tr, Unit *u) {
transaction_delete_job(tr, j, true);
}

void transaction_abort(Transaction *tr) {
static void transaction_abort(Transaction *tr) {
Job *j;

assert(tr);
Expand Down Expand Up @@ -1199,8 +1199,21 @@ Transaction *transaction_new(bool irreversible) {
return tr;
}

void transaction_free(Transaction *tr) {
Transaction *transaction_free(Transaction *tr) {
if (!tr)
return NULL;

assert(hashmap_isempty(tr->jobs));
hashmap_free(tr->jobs);
free(tr);

return mfree(tr);
}

Transaction *transaction_abort_and_free(Transaction *tr) {
if (!tr)
return NULL;

transaction_abort(tr);

return transaction_free(tr);
}
5 changes: 3 additions & 2 deletions src/core/transaction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ struct Transaction {
};

Transaction *transaction_new(bool irreversible);
void transaction_free(Transaction *tr);
Transaction *transaction_free(Transaction *tr);
Transaction *transaction_abort_and_free(Transaction *tr);
DEFINE_TRIVIAL_CLEANUP_FUNC(Transaction*, transaction_abort_and_free);

void transaction_add_propagate_reload_jobs(Transaction *tr, Unit *unit, Job *by, bool ignore_order, sd_bus_error *e);
int transaction_add_job_and_dependencies(
Expand All @@ -32,4 +34,3 @@ int transaction_add_job_and_dependencies(
int transaction_activate(Transaction *tr, Manager *m, JobMode mode, Set *affected, sd_bus_error *e);
int transaction_add_isolate_jobs(Transaction *tr, Manager *m);
int transaction_add_triggering_jobs(Transaction *tr, Unit *u);
void transaction_abort(Transaction *tr);
Loading

0 comments on commit 75fd8ad

Please sign in to comment.