X Tutup
Skip to content

Commit 99aad9a

Browse files
keszybzkeszybz
committed
systemctl: fix silent failure when --root is not found
Some calls to lookup_path_init() were not followed by any log emission. E.g.: $ SYSTEMD_LOG_LEVEL=debug systemctl --root=/missing enable unit; echo $? 1 Let's add a helper function and use it in various places. $ SYSTEMD_LOG_LEVEL=debug build/systemctl --root=/missing enable unit; echo $? Failed to initialize unit search paths for root directory /missing: No such file or directory 1 $ SYSTEMCTL_SKIP_SYSV=1 build/systemctl --root=/missing enable unit; echo $? Failed to initialize unit search paths for root directory /missing: No such file or directory Failed to enable: No such file or directory. 1 The repeated error in the second case is not very nice, but this is a niche case and I don't think it's worth the trouble to trying to avoid it.
1 parent 0d11db5 commit 99aad9a

File tree

14 files changed

+88
-105
lines changed

14 files changed

+88
-105
lines changed

‎src/analyze/analyze-unit-files.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ int verb_unit_files(int argc, char *argv[], void *userdata) {
2121
char **v;
2222
int r;
2323

24-
r = lookup_paths_init(&lp, arg_scope, 0, NULL);
24+
r = lookup_paths_init_or_warn(&lp, arg_scope, 0, NULL);
2525
if (r < 0)
26-
return log_error_errno(r, "lookup_paths_init() failed: %m");
26+
return r;
2727

2828
r = unit_file_build_name_map(&lp, NULL, &unit_ids, &unit_names, NULL);
2929
if (r < 0)

‎src/analyze/analyze-unit-paths.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ int verb_unit_paths(int argc, char *argv[], void *userdata) {
99
_cleanup_(lookup_paths_free) LookupPaths paths = {};
1010
int r;
1111

12-
r = lookup_paths_init(&paths, arg_scope, 0, NULL);
12+
r = lookup_paths_init_or_warn(&paths, arg_scope, 0, NULL);
1313
if (r < 0)
14-
return log_error_errno(r, "lookup_paths_init() failed: %m");
14+
return r;
1515

1616
STRV_FOREACH(p, paths.search_path)
1717
puts(*p);

‎src/basic/env-file.c‎

Lines changed: 28 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ static int parse_env_file_internal(
1616
FILE *f,
1717
const char *fname,
1818
int (*push) (const char *filename, unsigned line,
19-
const char *key, char *value, void *userdata, int *n_pushed),
20-
void *userdata,
21-
int *n_pushed) {
19+
const char *key, char *value, void *userdata),
20+
void *userdata) {
2221

2322
size_t n_key = 0, n_value = 0, last_value_whitespace = SIZE_MAX, last_key_whitespace = SIZE_MAX;
2423
_cleanup_free_ char *contents = NULL, *key = NULL, *value = NULL;
@@ -99,7 +98,7 @@ static int parse_env_file_internal(
9998
if (last_key_whitespace != SIZE_MAX)
10099
key[last_key_whitespace] = 0;
101100

102-
r = push(fname, line, key, value, userdata, n_pushed);
101+
r = push(fname, line, key, value, userdata);
103102
if (r < 0)
104103
return r;
105104

@@ -142,7 +141,7 @@ static int parse_env_file_internal(
142141
if (last_key_whitespace != SIZE_MAX)
143142
key[last_key_whitespace] = 0;
144143

145-
r = push(fname, line, key, value, userdata, n_pushed);
144+
r = push(fname, line, key, value, userdata);
146145
if (r < 0)
147146
return r;
148147

@@ -261,7 +260,7 @@ static int parse_env_file_internal(
261260
if (last_key_whitespace != SIZE_MAX)
262261
key[last_key_whitespace] = 0;
263262

264-
r = push(fname, line, key, value, userdata, n_pushed);
263+
r = push(fname, line, key, value, userdata);
265264
if (r < 0)
266265
return r;
267266

@@ -299,8 +298,7 @@ static int check_utf8ness_and_warn(
299298
static int parse_env_file_push(
300299
const char *filename, unsigned line,
301300
const char *key, char *value,
302-
void *userdata,
303-
int *n_pushed) {
301+
void *userdata) {
304302

305303
const char *k;
306304
va_list aq, *ap = userdata;
@@ -322,9 +320,6 @@ static int parse_env_file_push(
322320
free(*v);
323321
*v = value;
324322

325-
if (n_pushed)
326-
(*n_pushed)++;
327-
328323
return 1;
329324
}
330325
}
@@ -340,16 +335,13 @@ int parse_env_filev(
340335
const char *fname,
341336
va_list ap) {
342337

343-
int r, n_pushed = 0;
338+
int r;
344339
va_list aq;
345340

346341
va_copy(aq, ap);
347-
r = parse_env_file_internal(f, fname, parse_env_file_push, &aq, &n_pushed);
342+
r = parse_env_file_internal(f, fname, parse_env_file_push, &aq);
348343
va_end(aq);
349-
if (r < 0)
350-
return r;
351-
352-
return n_pushed;
344+
return r;
353345
}
354346

355347
int parse_env_file_sentinel(
@@ -370,8 +362,7 @@ int parse_env_file_sentinel(
370362
static int load_env_file_push(
371363
const char *filename, unsigned line,
372364
const char *key, char *value,
373-
void *userdata,
374-
int *n_pushed) {
365+
void *userdata) {
375366
char ***m = userdata;
376367
char *p;
377368
int r;
@@ -388,34 +379,28 @@ static int load_env_file_push(
388379
if (r < 0)
389380
return r;
390381

391-
if (n_pushed)
392-
(*n_pushed)++;
393-
394382
free(value);
395383
return 0;
396384
}
397385

398386
int load_env_file(FILE *f, const char *fname, char ***rl) {
399-
char **m = NULL;
387+
_cleanup_strv_free_ char **m = NULL;
400388
int r;
401389

402-
r = parse_env_file_internal(f, fname, load_env_file_push, &m, NULL);
403-
if (r < 0) {
404-
strv_free(m);
390+
r = parse_env_file_internal(f, fname, load_env_file_push, &m);
391+
if (r < 0)
405392
return r;
406-
}
407393

408-
*rl = m;
394+
*rl = TAKE_PTR(m);
409395
return 0;
410396
}
411397

412398
static int load_env_file_push_pairs(
413399
const char *filename, unsigned line,
414400
const char *key, char *value,
415-
void *userdata,
416-
int *n_pushed) {
401+
void *userdata) {
402+
417403
char ***m = ASSERT_PTR(userdata);
418-
bool added = false;
419404
int r;
420405

421406
r = check_utf8ness_and_warn(filename, line, key, value);
@@ -426,49 +411,37 @@ static int load_env_file_push_pairs(
426411
for (char **t = *m; t && *t; t += 2)
427412
if (streq(t[0], key)) {
428413
if (value)
429-
r = free_and_replace(t[1], value);
414+
return free_and_replace(t[1], value);
430415
else
431-
r = free_and_strdup(t+1, "");
432-
goto finish;
416+
return free_and_strdup(t+1, "");
433417
}
434418

435419
r = strv_extend(m, key);
436420
if (r < 0)
437-
return -ENOMEM;
421+
return r;
438422

439423
if (value)
440-
r = strv_push(m, value);
424+
return strv_push(m, value);
441425
else
442-
r = strv_extend(m, "");
443-
added = true;
444-
finish:
445-
if (r < 0)
446-
return r;
447-
448-
if (n_pushed && added)
449-
(*n_pushed)++;
450-
return 0;
426+
return strv_extend(m, "");
451427
}
452428

453429
int load_env_file_pairs(FILE *f, const char *fname, char ***rl) {
454-
char **m = NULL;
430+
_cleanup_strv_free_ char **m = NULL;
455431
int r;
456432

457-
r = parse_env_file_internal(f, fname, load_env_file_push_pairs, &m, NULL);
458-
if (r < 0) {
459-
strv_free(m);
433+
r = parse_env_file_internal(f, fname, load_env_file_push_pairs, &m);
434+
if (r < 0)
460435
return r;
461-
}
462436

463-
*rl = m;
437+
*rl = TAKE_PTR(m);
464438
return 0;
465439
}
466440

467441
static int merge_env_file_push(
468442
const char *filename, unsigned line,
469443
const char *key, char *value,
470-
void *userdata,
471-
int *n_pushed) {
444+
void *userdata) {
472445

473446
char ***env = userdata;
474447
char *expanded_value;
@@ -497,7 +470,7 @@ static int merge_env_file_push(
497470

498471
log_debug("%s:%u: setting %s=%s", filename, line, key, value);
499472

500-
return load_env_file_push(filename, line, key, value, env, n_pushed);
473+
return load_env_file_push(filename, line, key, value, env);
501474
}
502475

503476
int merge_env_file(
@@ -509,7 +482,7 @@ int merge_env_file(
509482
* plus "extended" substitutions, unlike other exported parsing functions.
510483
*/
511484

512-
return parse_env_file_internal(f, fname, merge_env_file_push, env, NULL);
485+
return parse_env_file_internal(f, fname, merge_env_file_push, env);
513486
}
514487

515488
static void write_env_var(FILE *f, const char *v) {

‎src/basic/path-lookup.c‎

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ static int get_paths_from_environ(const char *var, char ***paths, bool *append)
508508
}
509509

510510
int lookup_paths_init(
511-
LookupPaths *p,
511+
LookupPaths *lp,
512512
UnitFileScope scope,
513513
LookupPathsFlags flags,
514514
const char *root_dir) {
@@ -526,7 +526,7 @@ int lookup_paths_init(
526526
_cleanup_strv_free_ char **paths = NULL;
527527
int r;
528528

529-
assert(p);
529+
assert(lp);
530530
assert(scope >= 0);
531531
assert(scope < _UNIT_FILE_SCOPE_MAX);
532532

@@ -716,7 +716,7 @@ int lookup_paths_init(
716716
if (r < 0)
717717
return -ENOMEM;
718718

719-
*p = (LookupPaths) {
719+
*lp = (LookupPaths) {
720720
.search_path = strv_uniq(TAKE_PTR(paths)),
721721

722722
.persistent_config = TAKE_PTR(persistent_config),
@@ -741,41 +741,51 @@ int lookup_paths_init(
741741
return 0;
742742
}
743743

744-
void lookup_paths_free(LookupPaths *p) {
745-
if (!p)
744+
int lookup_paths_init_or_warn(LookupPaths *lp, UnitFileScope scope, LookupPathsFlags flags, const char *root_dir) {
745+
int r;
746+
747+
r = lookup_paths_init(lp, scope, flags, root_dir);
748+
if (r < 0)
749+
return log_error_errno(r, "Failed to initialize unit search paths%s%s: %m",
750+
isempty(root_dir) ? "" : " for root directory ", strempty(root_dir));
751+
return r;
752+
}
753+
754+
void lookup_paths_free(LookupPaths *lp) {
755+
if (!lp)
746756
return;
747757

748-
p->search_path = strv_free(p->search_path);
758+
lp->search_path = strv_free(lp->search_path);
749759

750-
p->persistent_config = mfree(p->persistent_config);
751-
p->runtime_config = mfree(p->runtime_config);
760+
lp->persistent_config = mfree(lp->persistent_config);
761+
lp->runtime_config = mfree(lp->runtime_config);
752762

753-
p->persistent_attached = mfree(p->persistent_attached);
754-
p->runtime_attached = mfree(p->runtime_attached);
763+
lp->persistent_attached = mfree(lp->persistent_attached);
764+
lp->runtime_attached = mfree(lp->runtime_attached);
755765

756-
p->generator = mfree(p->generator);
757-
p->generator_early = mfree(p->generator_early);
758-
p->generator_late = mfree(p->generator_late);
766+
lp->generator = mfree(lp->generator);
767+
lp->generator_early = mfree(lp->generator_early);
768+
lp->generator_late = mfree(lp->generator_late);
759769

760-
p->transient = mfree(p->transient);
770+
lp->transient = mfree(lp->transient);
761771

762-
p->persistent_control = mfree(p->persistent_control);
763-
p->runtime_control = mfree(p->runtime_control);
772+
lp->persistent_control = mfree(lp->persistent_control);
773+
lp->runtime_control = mfree(lp->runtime_control);
764774

765-
p->root_dir = mfree(p->root_dir);
766-
p->temporary_dir = mfree(p->temporary_dir);
775+
lp->root_dir = mfree(lp->root_dir);
776+
lp->temporary_dir = mfree(lp->temporary_dir);
767777
}
768778

769-
void lookup_paths_log(LookupPaths *p) {
770-
assert(p);
779+
void lookup_paths_log(LookupPaths *lp) {
780+
assert(lp);
771781

772-
if (strv_isempty(p->search_path)) {
782+
if (strv_isempty(lp->search_path)) {
773783
log_debug("Ignoring unit files.");
774-
p->search_path = strv_free(p->search_path);
784+
lp->search_path = strv_free(lp->search_path);
775785
} else {
776786
_cleanup_free_ char *t = NULL;
777787

778-
t = strv_join(p->search_path, "\n\t");
788+
t = strv_join(lp->search_path, "\n\t");
779789
log_debug("Looking for unit files in (higher priority first):\n\t%s", strna(t));
780790
}
781791
}

‎src/basic/path-lookup.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ struct LookupPaths {
5454
char *temporary_dir;
5555
};
5656

57-
int lookup_paths_init(LookupPaths *p, UnitFileScope scope, LookupPathsFlags flags, const char *root_dir);
57+
int lookup_paths_init(LookupPaths *lp, UnitFileScope scope, LookupPathsFlags flags, const char *root_dir);
58+
int lookup_paths_init_or_warn(LookupPaths *lp, UnitFileScope scope, LookupPathsFlags flags, const char *root_dir);
5859

5960
int xdg_user_dirs(char ***ret_config_dirs, char ***ret_data_dirs);
6061
int xdg_user_runtime_dir(char **ret, const char *suffix);

‎src/core/manager.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,11 +1751,11 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds, const char *roo
17511751

17521752
/* If we are running in test mode, we still want to run the generators,
17531753
* but we should not touch the real generator directories. */
1754-
r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope,
1755-
MANAGER_IS_TEST_RUN(m) ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0,
1756-
root);
1754+
r = lookup_paths_init_or_warn(&m->lookup_paths, m->unit_file_scope,
1755+
MANAGER_IS_TEST_RUN(m) ? LOOKUP_PATHS_TEMPORARY_GENERATED : 0,
1756+
root);
17571757
if (r < 0)
1758-
return log_error_errno(r, "Failed to initialize path lookup table: %m");
1758+
return r;
17591759

17601760
dual_timestamp_get(m->timestamps + manager_timestamp_initrd_mangle(MANAGER_TIMESTAMP_GENERATORS_START));
17611761
r = manager_run_environment_generators(m);
@@ -3343,9 +3343,9 @@ int manager_reload(Manager *m) {
33433343
m->uid_refs = hashmap_free(m->uid_refs);
33443344
m->gid_refs = hashmap_free(m->gid_refs);
33453345

3346-
r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope, 0, NULL);
3346+
r = lookup_paths_init_or_warn(&m->lookup_paths, m->unit_file_scope, 0, NULL);
33473347
if (r < 0)
3348-
log_warning_errno(r, "Failed to initialize path lookup table, ignoring: %m");
3348+
return r;
33493349

33503350
(void) manager_run_environment_generators(m);
33513351
(void) manager_run_generators(m);

‎src/shared/condition.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,8 @@ static int condition_test_needs_update(Condition *c, char **env) {
785785
if (r < 0) {
786786
log_debug_errno(r, "Failed to parse timestamp file '%s', using mtime: %m", p);
787787
return true;
788-
} else if (r == 0) {
788+
}
789+
if (isempty(timestamp_str)) {
789790
log_debug("No data in timestamp file '%s', using mtime.", p);
790791
return true;
791792
}

‎src/shared/install.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2597,7 +2597,7 @@ int unit_file_enable(
25972597
assert(scope >= 0);
25982598
assert(scope < _UNIT_FILE_SCOPE_MAX);
25992599

2600-
r = lookup_paths_init(&lp, scope, 0, root_dir);
2600+
r = lookup_paths_init_or_warn(&lp, scope, 0, root_dir);
26012601
if (r < 0)
26022602
return r;
26032603

0 commit comments

Comments
 (0)
X Tutup