From 785d345145bbd06c8f1c75c6a0b119c4e8f411db Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 1 Feb 2017 09:04:27 -0500 Subject: [PATCH] Trivial typo fixes and code refactorings (#5191) * logind: trivial simplification free_and_strdup() handles NULL arg, so make use of that. * boot: fix two typos * pid1: rewrite check in ignore_proc() to not check condition twice It's harmless, but it seems nicer to evaluate a condition just a single time. * core/execute: reformat exec_context_named_iofds() for legibility * core/execute.c: check asprintf return value in the usual fashion This is unlikely to fail, but we cannot rely on asprintf return value on failure, so let's just be correct here. CID #1368227. * core/timer: use (void) CID #1368234. * journal-file: check asprintf return value in the usual fashion This is unlikely to fail, but we cannot rely on asprintf return value on failure, so let's just be correct here. CID #1368236. * shared/cgroup-show: use (void) CID #1368243. * cryptsetup: do not return uninitialized value on error CID #1368416. --- src/boot/efi/boot.c | 2 +- src/boot/efi/stub.c | 2 +- src/core/execute.c | 62 ++++++++++++++++++++++++++++++--------------- src/core/killall.c | 29 ++++++++++----------- src/core/timer.c | 2 +- src/cryptsetup/cryptsetup.c | 4 +-- src/journal/journal-file.c | 17 ++++++++----- src/login/logind-dbus.c | 10 +++----- src/shared/cgroup-show.c | 2 +- 9 files changed, 73 insertions(+), 57 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 44ea621..7cc54a8 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1647,7 +1647,7 @@ static EFI_STATUS image_start(EFI_HANDLE parent_image, const Config *config, con loaded_image->LoadOptionsSize = (StrLen(loaded_image->LoadOptions)+1) * sizeof(CHAR16); #ifdef SD_BOOT_LOG_TPM - /* Try to log any options to the TPM, escpecially to catch manually edited options */ + /* Try to log any options to the TPM, especially to catch manually edited options */ err = tpm_log_event(SD_TPM_PCR, (EFI_PHYSICAL_ADDRESS) loaded_image->LoadOptions, loaded_image->LoadOptionsSize, loaded_image->LoadOptions); diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index 1e250f3..7c1ffb1 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -100,7 +100,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { cmdline = line; #ifdef SD_BOOT_LOG_TPM - /* Try to log any options to the TPM, escpecially manually edited options */ + /* Try to log any options to the TPM, especially manually edited options */ err = tpm_log_event(SD_TPM_PCR, (EFI_PHYSICAL_ADDRESS) loaded_image->LoadOptions, loaded_image->LoadOptionsSize, loaded_image->LoadOptions); diff --git a/src/core/execute.c b/src/core/execute.c index d0faba5..47cc431 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1685,25 +1685,31 @@ static int setup_private_users(uid_t uid, gid_t gid) { * child then writes the UID mapping, under full privileges. The parent waits for the child to finish and * continues execution normally. */ - if (uid != 0 && uid_is_valid(uid)) - asprintf(&uid_map, - "0 0 1\n" /* Map root → root */ - UID_FMT " " UID_FMT " 1\n", /* Map $UID → $UID */ - uid, uid); - else + if (uid != 0 && uid_is_valid(uid)) { + r = asprintf(&uid_map, + "0 0 1\n" /* Map root → root */ + UID_FMT " " UID_FMT " 1\n", /* Map $UID → $UID */ + uid, uid); + if (r < 0) + return -ENOMEM; + } else { uid_map = strdup("0 0 1\n"); /* The case where the above is the same */ - if (!uid_map) - return -ENOMEM; + if (!uid_map) + return -ENOMEM; + } - if (gid != 0 && gid_is_valid(gid)) - asprintf(&gid_map, - "0 0 1\n" /* Map root → root */ - GID_FMT " " GID_FMT " 1\n", /* Map $GID → $GID */ - gid, gid); - else + if (gid != 0 && gid_is_valid(gid)) { + r = asprintf(&gid_map, + "0 0 1\n" /* Map root → root */ + GID_FMT " " GID_FMT " 1\n", /* Map $GID → $GID */ + gid, gid); + if (r < 0) + return -ENOMEM; + } else { gid_map = strdup("0 0 1\n"); /* The case where the above is the same */ - if (!gid_map) - return -ENOMEM; + if (!gid_map) + return -ENOMEM; + } /* Create a communication channel so that the parent can tell the child when it finished creating the user * namespace. */ @@ -3096,7 +3102,7 @@ const char* exec_context_fdname(const ExecContext *c, int fd_index) { int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParameters *p, int named_iofds[3]) { unsigned i, targets; - const char *stdio_fdname[3]; + const char* stdio_fdname[3]; assert(c); assert(p); @@ -3109,18 +3115,32 @@ int exec_context_named_iofds(Unit *unit, const ExecContext *c, const ExecParamet stdio_fdname[i] = exec_context_fdname(c, i); for (i = 0; i < p->n_fds && targets > 0; i++) - if (named_iofds[STDIN_FILENO] < 0 && c->std_input == EXEC_INPUT_NAMED_FD && stdio_fdname[STDIN_FILENO] && streq(p->fd_names[i], stdio_fdname[STDIN_FILENO])) { + if (named_iofds[STDIN_FILENO] < 0 && + c->std_input == EXEC_INPUT_NAMED_FD && + stdio_fdname[STDIN_FILENO] && + streq(p->fd_names[i], stdio_fdname[STDIN_FILENO])) { + named_iofds[STDIN_FILENO] = p->fds[i]; targets--; - } else if (named_iofds[STDOUT_FILENO] < 0 && c->std_output == EXEC_OUTPUT_NAMED_FD && stdio_fdname[STDOUT_FILENO] && streq(p->fd_names[i], stdio_fdname[STDOUT_FILENO])) { + + } else if (named_iofds[STDOUT_FILENO] < 0 && + c->std_output == EXEC_OUTPUT_NAMED_FD && + stdio_fdname[STDOUT_FILENO] && + streq(p->fd_names[i], stdio_fdname[STDOUT_FILENO])) { + named_iofds[STDOUT_FILENO] = p->fds[i]; targets--; - } else if (named_iofds[STDERR_FILENO] < 0 && c->std_error == EXEC_OUTPUT_NAMED_FD && stdio_fdname[STDERR_FILENO] && streq(p->fd_names[i], stdio_fdname[STDERR_FILENO])) { + + } else if (named_iofds[STDERR_FILENO] < 0 && + c->std_error == EXEC_OUTPUT_NAMED_FD && + stdio_fdname[STDERR_FILENO] && + streq(p->fd_names[i], stdio_fdname[STDERR_FILENO])) { + named_iofds[STDERR_FILENO] = p->fds[i]; targets--; } - return (targets == 0 ? 0 : -ENOENT); + return targets == 0 ? 0 : -ENOENT; } int exec_context_load_environment(Unit *unit, const ExecContext *c, char ***l) { diff --git a/src/core/killall.c b/src/core/killall.c index b3aa22a..7a9df54 100644 --- a/src/core/killall.c +++ b/src/core/killall.c @@ -66,29 +66,26 @@ static bool ignore_proc(pid_t pid, bool warn_rootfs) { if (count <= 0) return true; - /* Processes with argv[0][0] = '@' we ignore from the killing - * spree. + /* Processes with argv[0][0] = '@' we ignore from the killing spree. * * http://www.freedesktop.org/wiki/Software/systemd/RootStorageDaemons */ - if (c == '@' && warn_rootfs) { - _cleanup_free_ char *comm = NULL; + if (c != '@') + return false; - r = pid_from_same_root_fs(pid); - if (r < 0) - return true; + if (warn_rootfs && + pid_from_same_root_fs(pid) == 0) { + + _cleanup_free_ char *comm = NULL; get_process_comm(pid, &comm); - if (r) - log_notice("Process " PID_FMT " (%s) has been marked to be excluded from killing. It is " - "running from the root file system, and thus likely to block re-mounting of the " - "root file system to read-only. Please consider moving it into an initrd file " - "system instead.", pid, strna(comm)); - return true; - } else if (c == '@') - return true; + log_notice("Process " PID_FMT " (%s) has been marked to be excluded from killing. It is " + "running from the root file system, and thus likely to block re-mounting of the " + "root file system to read-only. Please consider moving it into an initrd file " + "system instead.", pid, strna(comm)); + } - return false; + return true; } static void wait_for_children(Set *pids, sigset_t *mask) { diff --git a/src/core/timer.c b/src/core/timer.c index 5ee1466..d7441d6 100644 --- a/src/core/timer.c +++ b/src/core/timer.c @@ -232,7 +232,7 @@ static void timer_dump(Unit *u, FILE *f, const char *prefix) { if (v->base == TIMER_CALENDAR) { _cleanup_free_ char *p = NULL; - calendar_spec_to_string(v->calendar_spec, &p); + (void) calendar_spec_to_string(v->calendar_spec, &p); fprintf(f, "%s%s: %s\n", diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index cefd1b8..91c6533 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -594,7 +594,7 @@ static int help(void) { int main(int argc, char *argv[]) { struct crypt_device *cd = NULL; - int r; + int r = -EINVAL; if (argc <= 1) { r = help(); @@ -603,7 +603,6 @@ int main(int argc, char *argv[]) { if (argc < 3) { log_error("This program requires at least two arguments."); - r = -EINVAL; goto finish; } @@ -750,7 +749,6 @@ int main(int argc, char *argv[]) { } else { log_error("Unknown verb %s.", argv[1]); - r = -EINVAL; goto finish; } diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index e45d190..0a264ae 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -3087,13 +3087,18 @@ int journal_file_open( } } - if (fname) + if (fname) { f->path = strdup(fname); - else /* If we don't know the path, fill in something explanatory and vaguely useful */ - asprintf(&f->path, "/proc/self/%i", fd); - if (!f->path) { - r = -ENOMEM; - goto fail; + if (!f->path) { + r = -ENOMEM; + goto fail; + } + } else { + /* If we don't know the path, fill in something explanatory and vaguely useful */ + if (asprintf(&f->path, "/proc/self/%i", fd) < 0) { + r = -ENOMEM; + goto fail; + } } f->chain_cache = ordered_hashmap_new(&uint64_hash_ops); diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 3873bf3..ad44ca2 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2396,13 +2396,9 @@ static int method_set_wall_message( if (r == 0) return 1; /* Will call us back */ - if (isempty(wall_message)) - m->wall_message = mfree(m->wall_message); - else { - r = free_and_strdup(&m->wall_message, wall_message); - if (r < 0) - return log_oom(); - } + r = free_and_strdup(&m->wall_message, empty_to_null(wall_message)); + if (r < 0) + return log_oom(); m->enable_wall_messages = enable_wall_messages; diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index 5d21742..f5bb060 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -73,7 +73,7 @@ static void show_pid_array( for (i = 0; i < n_pids; i++) { _cleanup_free_ char *t = NULL; - get_process_cmdline(pids[i], n_columns, true, &t); + (void) get_process_cmdline(pids[i], n_columns, true, &t); if (extra) printf("%s%s ", prefix, special_glyph(TRIANGULAR_BULLET)); -- 2.7.4