From 1696bc55f9980314935d29e22decd73f3cbc1acf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jan 2025 23:45:43 +0100 Subject: [PATCH 1/4] sysusers: rework --help to do sections, and ANSI highlighting --- src/sysusers/sysusers.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 44253483db..eff9d1ca06 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -2014,12 +2014,14 @@ static int help(void) { if (r < 0) return log_oom(); - printf("%s [OPTIONS...] [CONFIGURATION FILE...]\n\n" - "Creates system user accounts.\n\n" - " -h --help Show this help\n" - " --version Show package version\n" + printf("%1$s [OPTIONS...] [CONFIGURATION FILE...]\n" + "\n%2$sCreates system user and group accounts.%4$s\n" + "\n%3$sCommands:%4$s\n" " --cat-config Show configuration files\n" " --tldr Show non-comment parts of configuration\n" + " -h --help Show this help\n" + " --version Show package version\n" + "\n%3$sOptions:%4$s\n" " --root=PATH Operate on an alternate filesystem root\n" " --image=PATH Operate on disk image as filesystem root\n" " --image-policy=POLICY Specify disk image dissection policy\n" @@ -2027,8 +2029,11 @@ static int help(void) { " --dry-run Just print what would be done\n" " --inline Treat arguments as configuration lines\n" " --no-pager Do not pipe output into a pager\n" - "\nSee the %s for details.\n", + "\nSee the %5$s for details.\n", program_invocation_short_name, + ansi_highlight(), + ansi_underline(), + ansi_normal(), link); return 0; From ca7d68f7f0cfebcfe512e6f2080679f95b739639 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jan 2025 23:48:53 +0100 Subject: [PATCH 2/4] sysusers: modernize loading of user/group databases Let's use chase(), and let's allocate sets/hashmaps on-the-fly. --- src/sysusers/sysusers.c | 57 +++++++++++++---------------------------- 1 file changed, 18 insertions(+), 39 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index eff9d1ca06..9929d1cd97 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -164,28 +164,16 @@ static void maybe_emit_login_defs_warning(Context *c) { } static int load_user_database(Context *c) { + _cleanup_free_ char *passwd_path = NULL; _cleanup_fclose_ FILE *f = NULL; - const char *passwd_path; struct passwd *pw; int r; assert(c); - passwd_path = prefix_roota(arg_root, "/etc/passwd"); - f = fopen(passwd_path, "re"); - if (!f) - return errno == ENOENT ? 0 : -errno; - - r = hashmap_ensure_allocated(&c->database_by_username, &string_hash_ops); - if (r < 0) - return r; - - r = hashmap_ensure_allocated(&c->database_by_uid, NULL); - if (r < 0) - return r; - - /* Note that we use NULL, i.e. trivial_hash_ops here, so identical strings can exist in the set. */ - r = set_ensure_allocated(&c->names, NULL); + r = chase_and_fopen_unlocked("/etc/passwd", arg_root, CHASE_PREFIX_ROOT, "re", &passwd_path, &f); + if (r == -ENOENT) + return 0; if (r < 0) return r; @@ -195,19 +183,21 @@ static int load_user_database(Context *c) { if (!n) return -ENOMEM; - r = set_consume(c->names, n); + /* Note that we use NULL hash_ops (i.e. trivial_hash_ops) here, so identical strings can + * exist in the set. */ + r = set_ensure_consume(&c->names, /* hash_ops= */ NULL, n); if (r < 0) return r; assert(r > 0); /* The set uses pointer comparisons, so n must not be in the set. */ - r = hashmap_put(c->database_by_username, n, UID_TO_PTR(pw->pw_uid)); + r = hashmap_ensure_put(&c->database_by_username, &string_hash_ops, n, UID_TO_PTR(pw->pw_uid)); if (r == -EEXIST) log_debug_errno(r, "%s: user '%s' is listed twice, ignoring duplicate uid.", passwd_path, n); else if (r < 0) return r; - r = hashmap_put(c->database_by_uid, UID_TO_PTR(pw->pw_uid), n); + r = hashmap_ensure_put(&c->database_by_uid, /* hash_ops= */ NULL, UID_TO_PTR(pw->pw_uid), n); if (r == -EEXIST) log_debug_errno(r, "%s: uid "UID_FMT" is listed twice, ignoring duplicate name.", passwd_path, pw->pw_uid); @@ -218,50 +208,39 @@ static int load_user_database(Context *c) { } static int load_group_database(Context *c) { + _cleanup_free_ char *group_path = NULL; _cleanup_fclose_ FILE *f = NULL; - const char *group_path; struct group *gr; int r; assert(c); - group_path = prefix_roota(arg_root, "/etc/group"); - f = fopen(group_path, "re"); - if (!f) - return errno == ENOENT ? 0 : -errno; - - r = hashmap_ensure_allocated(&c->database_by_groupname, &string_hash_ops); - if (r < 0) - return r; - - r = hashmap_ensure_allocated(&c->database_by_gid, NULL); - if (r < 0) - return r; - - /* Note that we use NULL, i.e. trivial_hash_ops here, so identical strings can exist in the set. */ - r = set_ensure_allocated(&c->names, NULL); + r = chase_and_fopen_unlocked("/etc/group", arg_root, CHASE_PREFIX_ROOT, "re", &group_path, &f); + if (r == -ENOENT) + return 0; if (r < 0) return r; while ((r = fgetgrent_sane(f, &gr)) > 0) { - char *n = strdup(gr->gr_name); if (!n) return -ENOMEM; - r = set_consume(c->names, n); + /* Note that we use NULL hash_ops (i.e. trivial_hash_ops) here, so identical strings can + * exist in the set. */ + r = set_ensure_consume(&c->names, /* hash_ops= */ NULL, n); if (r < 0) return r; assert(r > 0); /* The set uses pointer comparisons, so n must not be in the set. */ - r = hashmap_put(c->database_by_groupname, n, GID_TO_PTR(gr->gr_gid)); + r = hashmap_ensure_put(&c->database_by_groupname, &string_hash_ops, n, GID_TO_PTR(gr->gr_gid)); if (r == -EEXIST) log_debug_errno(r, "%s: group '%s' is listed twice, ignoring duplicate gid.", group_path, n); else if (r < 0) return r; - r = hashmap_put(c->database_by_gid, GID_TO_PTR(gr->gr_gid), n); + r = hashmap_ensure_put(&c->database_by_gid, /* hash_ops= */ NULL, GID_TO_PTR(gr->gr_gid), n); if (r == -EEXIST) log_debug_errno(r, "%s: gid "GID_FMT" is listed twice, ignoring duplicate name.", group_path, gr->gr_gid); From 48b176cb2d355f05c5f4304a38a965f8a516e710 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jan 2025 23:50:10 +0100 Subject: [PATCH 3/4] sysusers: add missing assert() --- src/sysusers/sysusers.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 9929d1cd97..8dd2890299 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -404,6 +404,8 @@ static int putsgent_with_members( #endif static const char* pick_shell(const Item *i) { + assert(i); + if (i->type != ADD_USER) return NULL; if (i->shell) From 0fd2c1fa592794c3552bc79218455acfc70b9194 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jan 2025 23:50:41 +0100 Subject: [PATCH 4/4] sysusers: use chase_and_stat() where appropriate --- src/sysusers/sysusers.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index 8dd2890299..d4997bb6f0 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -1039,11 +1039,8 @@ static int uid_is_ok( return 1; } -static int root_stat(const char *p, struct stat *st) { - const char *fix; - - fix = prefix_roota(arg_root, p); - return RET_NERRNO(stat(fix, st)); +static int root_stat(const char *p, struct stat *ret_st) { + return chase_and_stat(p, arg_root, CHASE_PREFIX_ROOT, /* ret_path= */ NULL, ret_st); } static int read_id_from_file(Item *i, uid_t *ret_uid, gid_t *ret_gid) {