From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 11 Aug 2010 20:04:22 +0000 (+0200)
Subject: clang: fix numerous little issues found with clang-analyzer
X-Git-Tag: v8~121
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=e364ad0628b5930a671ae5be863b960f4bd748a8

clang: fix numerous little issues found with clang-analyzer
---

diff --git a/src/automount.c b/src/automount.c
index 9843481a6..047af7759 100644
--- a/src/automount.c
+++ b/src/automount.c
@@ -444,6 +444,8 @@ int automount_send_ready(Automount *a, int status) {
         else
                 log_debug("Sending success.");
 
+        r = 0;
+
         /* Autofs thankfully does not hand out 0 as a token */
         while ((token = PTR_TO_UINT(set_steal_first(a->tokens)))) {
                 int k;
@@ -460,8 +462,6 @@ int automount_send_ready(Automount *a, int status) {
                         r = k;
         }
 
-        r = 0;
-
 fail:
         if (ioctl_fd >= 0)
                 close_nointr_nofail(ioctl_fd);
diff --git a/src/cgroup-show.c b/src/cgroup-show.c
index 24d2ba398..af44729a5 100644
--- a/src/cgroup-show.c
+++ b/src/cgroup-show.c
@@ -138,7 +138,7 @@ static int show_cgroup_one_by_path(const char *path, const char *prefix, unsigne
                         printf("%s%s %*lu %s\n",
                                prefix,
                                (more || i < n-1) ? "\342\224\234" : "\342\224\224",
-                               ilog10(biggest),
+                               (int) ilog10(biggest),
                                (unsigned long) pids[i],
                                strna(t));
 
diff --git a/src/cgroup-util.c b/src/cgroup-util.c
index 2f3fcb598..88adec0c9 100644
--- a/src/cgroup-util.c
+++ b/src/cgroup-util.c
@@ -737,10 +737,8 @@ int cg_install_release_agent(const char *controller, const char *agent) {
 
         free(fs);
         fs = NULL;
-        if ((r = cg_get_path(controller, NULL, "notify_on_release", &fs)) < 0) {
-                r = -ENOMEM;
+        if ((r = cg_get_path(controller, NULL, "notify_on_release", &fs)) < 0)
                 goto finish;
-        }
 
         free(contents);
         contents = NULL;
diff --git a/src/cgroup.c b/src/cgroup.c
index 02c291918..23ba5d86a 100644
--- a/src/cgroup.c
+++ b/src/cgroup.c
@@ -231,7 +231,7 @@ int manager_setup_cgroup(Manager *m) {
         } else {
                 /* We need a new root cgroup */
                 m->cgroup_hierarchy = NULL;
-                if ((r = asprintf(&m->cgroup_hierarchy, "%s%s", streq(current, "/") ? "" : current, suffix)) < 0) {
+                if (asprintf(&m->cgroup_hierarchy, "%s%s", streq(current, "/") ? "" : current, suffix) < 0) {
                         r = -ENOMEM;
                         goto finish;
                 }
@@ -325,14 +325,13 @@ int cgroup_notify_empty(Manager *m, const char *group) {
 Unit* cgroup_unit_by_pid(Manager *m, pid_t pid) {
         CGroupBonding *l, *b;
         char *group = NULL;
-        int r;
 
         assert(m);
 
         if (pid <= 1)
                 return NULL;
 
-        if ((r = cg_get_by_pid(SYSTEMD_CGROUP_CONTROLLER, pid, &group)))
+        if (cg_get_by_pid(SYSTEMD_CGROUP_CONTROLLER, pid, &group) < 0)
                 return NULL;
 
         l = hashmap_get(m->cgroup_bondings, group);
diff --git a/src/dbus-execute.c b/src/dbus-execute.c
index bb660b185..55c61e593 100644
--- a/src/dbus-execute.c
+++ b/src/dbus-execute.c
@@ -286,9 +286,9 @@ int bus_execute_append_command(Manager *m, DBusMessageIter *i, const char *prope
                     !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_BOOLEAN, &c->ignore) ||
                     !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_UINT64, &c->exec_status.start_timestamp.realtime) ||
                     !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_UINT64, &c->exec_status.exit_timestamp.realtime) ||
-                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_UINT32, &c->exec_status.pid) ||
-                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_INT32, &c->exec_status.code) ||
-                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_INT32, &c->exec_status.status))
+                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_UINT32, &pid) ||
+                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_INT32, &code) ||
+                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_INT32, &status))
                         return -ENOMEM;
 
                 if (!dbus_message_iter_close_container(&sub, &sub2))
diff --git a/src/execute.c b/src/execute.c
index 352def3cf..b34c05201 100644
--- a/src/execute.c
+++ b/src/execute.c
@@ -1055,7 +1055,7 @@ int exec_spawn(ExecCommand *command,
                 }
 
                 if (cgroup_bondings)
-                        if ((r = cgroup_bonding_install_list(cgroup_bondings, 0)) < 0) {
+                        if (cgroup_bonding_install_list(cgroup_bondings, 0) < 0) {
                                 r = EXIT_CGROUP;
                                 goto fail;
                         }
diff --git a/src/initctl.c b/src/initctl.c
index 74eccac55..a504c8f33 100644
--- a/src/initctl.c
+++ b/src/initctl.c
@@ -387,7 +387,7 @@ int main(int argc, char *argv[]) {
                 if (k <= 0)
                         break;
 
-                if ((k = process_event(&server, &event)) < 0)
+                if (process_event(&server, &event) < 0)
                         goto fail;
         }
 
diff --git a/src/logger.c b/src/logger.c
index 3d69fcf2c..a9ac54b11 100644
--- a/src/logger.c
+++ b/src/logger.c
@@ -585,7 +585,7 @@ int main(int argc, char *argv[]) {
                 if (k <= 0)
                         break;
 
-                if ((k = process_event(&server, &event)) < 0)
+                if (process_event(&server, &event) < 0)
                         goto fail;
         }
 
diff --git a/src/main.c b/src/main.c
index 79a46b8cd..3810033fb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -117,10 +117,10 @@ _noreturn_ static void crash(int sig) {
                         _exit(1);
 
                 } else {
-                        int status, r;
+                        int status;
 
                         /* Order things nicely. */
-                        if ((r = waitpid(pid, &status, 0)) < 0)
+                        if (waitpid(pid, &status, 0) < 0)
                                 log_error("Caught <%s>, waitpid() failed: %s", signal_to_string(sig), strerror(errno));
                         else if (!WCOREDUMP(status))
                                 log_error("Caught <%s>, core dump failed.", signal_to_string(sig));
@@ -540,7 +540,7 @@ static int parse_proc_cmdline(void) {
         char *state;
 
         if ((r = read_one_line_file("/proc/cmdline", &line)) < 0) {
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(errno));
+                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
                 return 0;
         }
 
diff --git a/src/manager.c b/src/manager.c
index 18b014004..9684efac8 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -791,7 +791,8 @@ static int delete_one_unmergeable_job(Manager *m, Job *j) {
                                                 d = j;
                                         else
                                                 d = k;
-                                }
+                                } else
+                                        d = j;
 
                         } else if (!j->matters_to_anchor)
                                 d = j;
@@ -824,7 +825,7 @@ static int transaction_merge_jobs(Manager *m, DBusError *e) {
 
                 t = j->type;
                 LIST_FOREACH(transaction, k, j->transaction_next) {
-                        if ((r = job_type_merge(&t, k->type)) >= 0)
+                        if (job_type_merge(&t, k->type) >= 0)
                                 continue;
 
                         /* OK, we could not merge all jobs for this
@@ -1293,7 +1294,6 @@ rollback:
 
 static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool override, bool *is_new) {
         Job *j, *f;
-        int r;
 
         assert(m);
         assert(unit);
@@ -1326,7 +1326,7 @@ static Job* transaction_add_one_job(Manager *m, JobType type, Unit *unit, bool o
 
         LIST_PREPEND(Job, transaction, f, j);
 
-        if ((r = hashmap_replace(m->transaction_jobs, unit, f)) < 0) {
+        if (hashmap_replace(m->transaction_jobs, unit, f) < 0) {
                 job_free(j);
                 return NULL;
         }
diff --git a/src/mount.c b/src/mount.c
index cf2355a33..9a201f33e 100644
--- a/src/mount.c
+++ b/src/mount.c
@@ -789,6 +789,7 @@ static void mount_enter_remounting(Mount *m, bool success) {
         return;
 
 fail:
+        log_warning("%s failed to run 'remount' task: %s", m->meta.id, strerror(-r));
         mount_enter_mounted(m, false);
 }
 
@@ -876,7 +877,6 @@ static int mount_serialize(Unit *u, FILE *f, FDSet *fds) {
 
 static int mount_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) {
         Mount *m = MOUNT(u);
-        int r;
 
         assert(u);
         assert(key);
@@ -901,7 +901,7 @@ static int mount_deserialize_item(Unit *u, const char *key, const char *value, F
         } else if (streq(key, "control-pid")) {
                 pid_t pid;
 
-                if ((r = parse_pid(value, &pid)) < 0)
+                if (parse_pid(value, &pid) < 0)
                         log_debug("Failed to parse control-pid value %s", value);
                 else
                         m->control_pid = pid;
@@ -1422,7 +1422,7 @@ void mount_fd_event(Manager *m, int events) {
          * table changes */
 
         if ((r = mount_load_proc_self_mountinfo(m, true)) < 0) {
-                log_error("Failed to reread /proc/self/mountinfo: %s", strerror(errno));
+                log_error("Failed to reread /proc/self/mountinfo: %s", strerror(-r));
 
                 /* Reset flags, just in case, for later calls */
                 LIST_FOREACH(units_per_type, meta, m->units_per_type[UNIT_MOUNT]) {
diff --git a/src/namespace.c b/src/namespace.c
index 09bcaff96..9e964c5b8 100644
--- a/src/namespace.c
+++ b/src/namespace.c
@@ -151,6 +151,9 @@ static int apply_mount(Path *p, const char *root_dir, const char *inaccessible_d
         case PRIVATE:
                 what = private_dir;
                 break;
+
+        default:
+                assert_not_reached("Unknown mode");
         }
 
         if ((r = mount(what, where, NULL, MS_BIND|MS_REC, NULL)) >= 0) {
diff --git a/src/path.c b/src/path.c
index 91de56f34..2f1e0aea6 100644
--- a/src/path.c
+++ b/src/path.c
@@ -145,12 +145,10 @@ static int path_load(Unit *u) {
 
 static void path_dump(Unit *u, FILE *f, const char *prefix) {
         Path *p = PATH(u);
-        const char *prefix2;
-        char *p2;
         PathSpec *s;
 
-        p2 = strappend(prefix, "\t");
-        prefix2 = p2 ? p2 : prefix;
+        assert(p);
+        assert(f);
 
         fprintf(f,
                 "%sPath State: %s\n"
@@ -164,8 +162,6 @@ static void path_dump(Unit *u, FILE *f, const char *prefix) {
                         prefix,
                         path_type_to_string(s->type),
                         s->path);
-
-        free(p2);
 }
 
 static void path_unwatch_one(Path *p, PathSpec *s) {
diff --git a/src/service.c b/src/service.c
index 318d8a7dc..a3a23e327 100644
--- a/src/service.c
+++ b/src/service.c
@@ -797,7 +797,7 @@ static int service_load_sysv(Service *s) {
                         if (t == s->meta.id)
                                 continue;
 
-                        if ((r == service_load_sysv_name(s, t)) < 0)
+                        if ((r = service_load_sysv_name(s, t)) < 0)
                                 return r;
 
                         if (s->meta.load_state != UNIT_STUB)
@@ -2063,7 +2063,6 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) {
 
 static int service_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) {
         Service *s = SERVICE(u);
-        int r;
 
         assert(u);
         assert(key);
@@ -2087,14 +2086,14 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
         } else if (streq(key, "control-pid")) {
                 pid_t pid;
 
-                if ((r = parse_pid(value, &pid)) < 0)
+                if (parse_pid(value, &pid) < 0)
                         log_debug("Failed to parse control-pid value %s", value);
                 else
                         s->control_pid = pid;
         } else if (streq(key, "main-pid")) {
                 pid_t pid;
 
-                if ((r = parse_pid(value, &pid)) < 0)
+                if (parse_pid(value, &pid) < 0)
                         log_debug("Failed to parse main-pid value %s", value);
                 else
                         service_set_main_pid(s, (pid_t) pid);
@@ -2136,49 +2135,49 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
         } else if (streq(key, "main-exec-status-pid")) {
                 pid_t pid;
 
-                if ((r = parse_pid(value, &pid)) < 0)
+                if (parse_pid(value, &pid) < 0)
                         log_debug("Failed to parse main-exec-status-pid value %s", value);
                 else
                         s->main_exec_status.pid = pid;
         } else if (streq(key, "main-exec-status-code")) {
                 int i;
 
-                if ((r = safe_atoi(value, &i)) < 0)
+                if (safe_atoi(value, &i) < 0)
                         log_debug("Failed to parse main-exec-status-code value %s", value);
                 else
                         s->main_exec_status.code = i;
         } else if (streq(key, "main-exec-status-status")) {
                 int i;
 
-                if ((r = safe_atoi(value, &i)) < 0)
+                if (safe_atoi(value, &i) < 0)
                         log_debug("Failed to parse main-exec-status-status value %s", value);
                 else
                         s->main_exec_status.status = i;
         } else if (streq(key, "main-exec-status-start-realtime")) {
                 uint64_t k;
 
-                if ((r = safe_atou64(value, &k)) < 0)
+                if (safe_atou64(value, &k) < 0)
                         log_debug("Failed to parse main-exec-status-start-realtime value %s", value);
                 else
                         s->main_exec_status.start_timestamp.realtime = (usec_t) k;
         } else if (streq(key, "main-exec-status-start-monotonic")) {
                 uint64_t k;
 
-                if ((r = safe_atou64(value, &k)) < 0)
+                if (safe_atou64(value, &k) < 0)
                         log_debug("Failed to parse main-exec-status-start-monotonic value %s", value);
                 else
                         s->main_exec_status.start_timestamp.monotonic = (usec_t) k;
         } else if (streq(key, "main-exec-status-exit-realtime")) {
                 uint64_t k;
 
-                if ((r = safe_atou64(value, &k)) < 0)
+                if (safe_atou64(value, &k) < 0)
                         log_debug("Failed to parse main-exec-status-exit-realtime value %s", value);
                 else
                         s->main_exec_status.exit_timestamp.realtime = (usec_t) k;
         } else if (streq(key, "main-exec-status-exit-monotonic")) {
                 uint64_t k;
 
-                if ((r = safe_atou64(value, &k)) < 0)
+                if (safe_atou64(value, &k) < 0)
                         log_debug("Failed to parse main-exec-status-exit-monotonic value %s", value);
                 else
                         s->main_exec_status.exit_timestamp.monotonic = (usec_t) k;
diff --git a/src/socket.c b/src/socket.c
index 23658ac27..5132f8e70 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -445,14 +445,14 @@ static void socket_dump(Unit *u, FILE *f, const char *prefix) {
                 if (p->type == SOCKET_SOCKET) {
                         const char *t;
                         int r;
-                        char *k;
+                        char *k = NULL;
 
                         if ((r = socket_address_print(&p->address, &k)) < 0)
                                 t = strerror(-r);
                         else
                                 t = k;
 
-                        fprintf(f, "%s%s: %s\n", prefix, listen_lookup(p->address.type), k);
+                        fprintf(f, "%s%s: %s\n", prefix, listen_lookup(p->address.type), t);
                         free(k);
                 } else
                         fprintf(f, "%sListenFIFO: %s\n", prefix, p->path);
@@ -1374,7 +1374,6 @@ static int socket_serialize(Unit *u, FILE *f, FDSet *fds) {
 
 static int socket_deserialize_item(Unit *u, const char *key, const char *value, FDSet *fds) {
         Socket *s = SOCKET(u);
-        int r;
 
         assert(u);
         assert(key);
@@ -1399,14 +1398,14 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
         } else if (streq(key, "n-accepted")) {
                 unsigned k;
 
-                if ((r = safe_atou(value, &k)) < 0)
+                if (safe_atou(value, &k) < 0)
                         log_debug("Failed to parse n-accepted value %s", value);
                 else
                         s->n_accepted += k;
         } else if (streq(key, "control-pid")) {
                 pid_t pid;
 
-                if ((r = parse_pid(value, &pid)) < 0)
+                if (parse_pid(value, &pid) < 0)
                         log_debug("Failed to parse control-pid value %s", value);
                 else
                         s->control_pid = pid;
@@ -1665,7 +1664,7 @@ int socket_collect_fds(Socket *s, int **fds, unsigned *n_fds) {
                 if (p->fd >= 0)
                         rn_fds++;
 
-        if (!(rfds = new(int, rn_fds)) < 0)
+        if (!(rfds = new(int, rn_fds)))
                 return -ENOMEM;
 
         k = 0;
diff --git a/src/systemctl.c b/src/systemctl.c
index 8ef1adda5..993e1d655 100644
--- a/src/systemctl.c
+++ b/src/systemctl.c
@@ -457,6 +457,8 @@ static int dot_one(DBusConnection *bus, const char *name, const char *path) {
                 dbus_message_iter_next(&sub);
         }
 
+        r = 0;
+
 finish:
         if (m)
                 dbus_message_unref(m);
@@ -798,7 +800,7 @@ finish:
 }
 
 static bool need_daemon_reload(DBusConnection *bus, const char *unit) {
-        DBusMessage *m, *reply;
+        DBusMessage *m = NULL, *reply = NULL;
         dbus_bool_t b = FALSE;
         DBusMessageIter iter, sub;
         const char
@@ -3154,7 +3156,7 @@ static int remove_marked_symlinks_fd(int fd, const char *config_path, const char
                         free(p);
 
                         if (r == 0)
-                                q = r;
+                                r = q;
 
                 } else if (is_link) {
                         char *p, *dest, *c;
@@ -4530,11 +4532,10 @@ static int systemctl_main(DBusConnection *bus, int argc, char *argv[], DBusError
 }
 
 static int reload_with_fallback(DBusConnection *bus) {
-        int r;
 
         if (bus) {
                 /* First, try systemd via D-Bus. */
-                if ((r = daemon_reload(bus, NULL, 0)) > 0)
+                if (daemon_reload(bus, NULL, 0) > 0)
                         return 0;
         }
 
@@ -4550,22 +4551,21 @@ static int reload_with_fallback(DBusConnection *bus) {
 }
 
 static int start_with_fallback(DBusConnection *bus) {
-        int r;
 
         if (bus) {
                 /* First, try systemd via D-Bus. */
-                if ((r = start_unit(bus, NULL, 0)) > 0)
+                if (start_unit(bus, NULL, 0) > 0)
                         goto done;
         }
 
         /* Hmm, talking to systemd via D-Bus didn't work. Then
          * let's try to talk to Upstart via D-Bus. */
-        if ((r = talk_upstart()) > 0)
+        if (talk_upstart() > 0)
                 goto done;
 
         /* Nothing else worked, so let's try
          * /dev/initctl */
-        if ((r = talk_initctl()) != 0)
+        if (talk_initctl() != 0)
                 goto done;
 
         log_error("Failed to talk to init daemon.");
diff --git a/src/timer.c b/src/timer.c
index 2c21b4919..a63de9466 100644
--- a/src/timer.c
+++ b/src/timer.c
@@ -114,15 +114,10 @@ static int timer_load(Unit *u) {
 
 static void timer_dump(Unit *u, FILE *f, const char *prefix) {
         Timer *t = TIMER(u);
-        const char *prefix2;
-        char *p2;
         TimerValue *v;
         char
                 timespan1[FORMAT_TIMESPAN_MAX];
 
-        p2 = strappend(prefix, "\t");
-        prefix2 = p2 ? p2 : prefix;
-
         fprintf(f,
                 "%sTimer State: %s\n"
                 "%sUnit: %s\n",
@@ -135,8 +130,6 @@ static void timer_dump(Unit *u, FILE *f, const char *prefix) {
                         prefix,
                         timer_base_to_string(v->base),
                         strna(format_timespan(timespan1, sizeof(timespan1), v->value)));
-
-        free(p2);
 }
 
 static void timer_set_state(Timer *t, TimerState state) {
diff --git a/src/unit.c b/src/unit.c
index 59fc93a9b..3c2e97416 100644
--- a/src/unit.c
+++ b/src/unit.c
@@ -859,10 +859,10 @@ int unit_reload(Unit *u) {
                 return -EBADR;
 
         state = unit_active_state(u);
-        if (unit_active_state(u) == UNIT_RELOADING)
+        if (state == UNIT_RELOADING)
                 return -EALREADY;
 
-        if (unit_active_state(u) != UNIT_ACTIVE)
+        if (state != UNIT_ACTIVE)
                 return -ENOEXEC;
 
         unit_add_to_dbus_queue(u);