Merge pull request #8943 from keszybz/coverity-fixes

Coverity fixes

src/basic/format-table.c

@@ -648,6 +648,7 @@ int table_set_display(Table *t, size_t first_column, ...) {
                         break;
 
         }
+        va_end(ap);
 
         return 0;
 }
@@ -676,6 +677,7 @@ int table_set_sort(Table *t, size_t first_column, ...) {
                 if (column == (size_t) -1)
                         break;
         }
+        va_end(ap);
 
         return 0;
 }

src/basic/fs-util.c

@@ -919,25 +919,12 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
         return exists;
 
 chased_one:
-
         if (ret) {
                 char *c;
 
-                if (done) {
-                        if (todo) {
-                                c = strjoin(done, todo);
-                                if (!c)
-                                        return -ENOMEM;
-                        } else
-                                c = TAKE_PTR(done);
-                } else {
-                        if (todo)
-                                c = strdup(todo);
-                        else
-                                c = strdup("/");
-                        if (!c)
-                                return -ENOMEM;
-                }
+                c = strjoin(strempty(done), todo);
+                if (!c)
+                        return -ENOMEM;
 
                 *ret = c;
         }

src/basic/strbuf.h

@@ -11,6 +11,8 @@
 #include <stdint.h>
 #include <sys/types.h>
 
+#include "macro.h"
+
 struct strbuf {
         char *buf;
         size_t len;
@@ -40,3 +42,4 @@ struct strbuf *strbuf_new(void);
 ssize_t strbuf_add_string(struct strbuf *str, const char *s, size_t len);
 void strbuf_complete(struct strbuf *str);
 void strbuf_cleanup(struct strbuf *str);
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct strbuf*, strbuf_cleanup);

src/core/mount-setup.c

@@ -279,7 +279,7 @@ int mount_cgroup_controllers(char ***join_controllers) {
                         if (strv_find(*k, controller))
                                 break;
 
-                if (k && *k) {
+                if (*k) {
                         char **i, **j;
 
                         for (i = *k, j = *k; *i; i++) {

src/journal/catalog.c

@@ -448,7 +448,7 @@ static int64_t write_catalog(const char *database, struct strbuf *sb,
 int catalog_update(const char* database, const char* root, const char* const* dirs) {
         _cleanup_strv_free_ char **files = NULL;
         char **f;
-        struct strbuf *sb = NULL;
+        _cleanup_(strbuf_cleanupp) struct strbuf *sb = NULL;
         _cleanup_hashmap_free_free_free_ Hashmap *h = NULL;
         _cleanup_free_ CatalogItem *items = NULL;
         ssize_t offset;
@@ -461,38 +461,29 @@ int catalog_update(const char* database, const char* root, const char* const* di
 
         h = hashmap_new(&catalog_hash_ops);
         sb = strbuf_new();
-
-        if (!h || !sb) {
-                r = log_oom();
-                goto finish;
-        }
+        if (!h || !sb)
+                return log_oom();
 
         r = conf_files_list_strv(&files, ".catalog", root, 0, dirs);
-        if (r < 0) {
-                log_error_errno(r, "Failed to get catalog files: %m");
-                goto finish;
-        }
+        if (r < 0)
+                return log_error_errno(r, "Failed to get catalog files: %m");
 
         STRV_FOREACH(f, files) {
                 log_debug("Reading file '%s'", *f);
                 r = catalog_import_file(h, *f);
-                if (r < 0) {
-                        log_error_errno(r, "Failed to import file '%s': %m", *f);
-                        goto finish;
-                }
+                if (r < 0)
+                        return log_error_errno(r, "Failed to import file '%s': %m", *f);
         }
 
         if (hashmap_size(h) <= 0) {
                 log_info("No items in catalog.");
-                goto finish;
+                return 0;
         } else
                 log_debug("Found %u items in catalog.", hashmap_size(h));
 
         items = new(CatalogItem, hashmap_size(h));
-        if (!items) {
-                r = log_oom();
-                goto finish;
-        }
+        if (!items)
+                return log_oom();
 
         n = 0;
         HASHMAP_FOREACH_KEY(payload, i, h, j) {
@@ -501,10 +492,9 @@ int catalog_update(const char* database, const char* root, const char* const* di
                           isempty(i->language) ? "C" : i->language);
 
                 offset = strbuf_add_string(sb, payload, strlen(payload));
-                if (offset < 0) {
-                        r = log_oom();
-                        goto finish;
-                }
+                if (offset < 0)
+                        return log_oom();
+
                 i->offset = htole64((uint64_t) offset);
                 items[n++] = *i;
         }
@@ -516,17 +506,11 @@ int catalog_update(const char* database, const char* root, const char* const* di
 
         sz = write_catalog(database, sb, items, n);
         if (sz < 0)
-                r = log_error_errno(sz, "Failed to write %s: %m", database);
-        else {
-                r = 0;
-                log_debug("%s: wrote %u items, with %zu bytes of strings, %"PRIi64" total size.",
-                          database, n, sb->len, sz);
-        }
-
-finish:
-        strbuf_cleanup(sb);
+                return log_error_errno(sz, "Failed to write %s: %m", database);
 
-        return r;
+        log_debug("%s: wrote %u items, with %zu bytes of strings, %"PRIi64" total size.",
+                  database, n, sb->len, sz);
+        return 0;
 }
 
 static int open_mmap(const char *database, int *_fd, struct stat *_st, void **_p) {

src/journal/test-catalog.c

@@ -177,16 +177,16 @@ static void test_catalog_update(void) {
 
         /* Test what happens if there are no files. */
         r = catalog_update(database, NULL, NULL);
-        assert_se(r >= 0);
+        assert_se(r == 0);
 
         /* Test what happens if there are no files in the directory. */
         r = catalog_update(database, NULL, no_catalog_dirs);
-        assert_se(r >= 0);
+        assert_se(r == 0);
 
         /* Make sure that we at least have some files loaded or the
            catalog_list below will fail. */
         r = catalog_update(database, NULL, catalog_dirs);
-        assert_se(r >= 0);
+        assert_se(r == 0);
 }
 
 static void test_catalog_file_lang(void) {

src/locale/keymap-util.c

@@ -530,9 +530,10 @@ int find_converted_keymap(const char *x11_layout, const char *x11_variant, char
         return 0;
 }
 
-int find_legacy_keymap(Context *c, char **new_keymap) {
+int find_legacy_keymap(Context *c, char **ret) {
         const char *map;
         _cleanup_fclose_ FILE *f = NULL;
+        _cleanup_free_ char *new_keymap = NULL;
         unsigned n = 0;
         unsigned best_matching = 0;
         int r;
@@ -597,7 +598,7 @@ int find_legacy_keymap(Context *c, char **new_keymap) {
                         if (matching > best_matching) {
                                 best_matching = matching;
 
-                                r = free_and_strdup(new_keymap, a[0]);
+                                r = free_and_strdup(&new_keymap, a[0]);
                                 if (r < 0)
                                         return r;
                         }
@@ -617,13 +618,12 @@ int find_legacy_keymap(Context *c, char **new_keymap) {
                 r = find_converted_keymap(l, v, &converted);
                 if (r < 0)
                         return r;
-                if (r > 0) {
-                        free(*new_keymap);
-                        *new_keymap = converted;
-                }
+                if (r > 0)
+                        free_and_replace(new_keymap, converted);
         }
 
-        return (bool) *new_keymap;
+        *ret = TAKE_PTR(new_keymap);
+        return (bool) *ret;
 }
 
 int find_language_fallback(const char *lang, char **language) {

src/login/logind.c

@@ -389,14 +389,18 @@ static int parse_fdname(const char *fdname, char **session_id, dev_t *dev) {
 
         if (!streq(parts[0], "session"))
                 return -EINVAL;
+
         id = strdup(parts[1]);
         if (!id)
                 return -ENOMEM;
 
         if (!streq(parts[2], "device"))
                 return -EINVAL;
-        r = safe_atou(parts[3], &major) ||
-            safe_atou(parts[4], &minor);
+
+        r = safe_atou(parts[3], &major);
+        if (r < 0)
+                return r;
+        r = safe_atou(parts[4], &minor);
         if (r < 0)
                 return r;
 

src/shared/sleep-config.c

@@ -34,7 +34,7 @@ int parse_sleep_config(const char *verb, char ***_modes, char ***_states, usec_t
                 **suspend_mode = NULL, **suspend_state = NULL,
                 **hibernate_mode = NULL, **hibernate_state = NULL,
                 **hybrid_mode = NULL, **hybrid_state = NULL;
-        char **modes, **states;
+        _cleanup_strv_free_ char **modes, **states; /* always initialized below */
         usec_t delay = 180 * USEC_PER_MINUTE;
 
         const ConfigTableItem items[] = {
@@ -90,16 +90,13 @@ int parse_sleep_config(const char *verb, char ***_modes, char ***_states, usec_t
                 assert_not_reached("what verb");
 
         if ((!modes && STR_IN_SET(verb, "hibernate", "hybrid-sleep")) ||
-            (!states && !streq(verb, "suspend-then-hibernate"))) {
-                strv_free(modes);
-                strv_free(states);
+            (!states && !streq(verb, "suspend-then-hibernate")))
                 return log_oom();
-        }
 
         if (_modes)
-                *_modes = modes;
+                *_modes = TAKE_PTR(modes);
         if (_states)
-                *_states = states;
+                *_states = TAKE_PTR(states);
         if (_delay)
                 *_delay = delay;
 

src/test/test-sleep.c

@@ -14,6 +14,13 @@
 #include "strv.h"
 #include "util.h"
 
+static void test_parse_sleep_config(void) {
+        const char *verb;
+
+        FOREACH_STRING(verb, "suspend", "hibernate", "hybrid-sleep", "suspend-then-hibernate")
+                assert_se(parse_sleep_config(verb, NULL, NULL, NULL) == 0);
+}
+
 static int test_fiemap(const char *path) {
         _cleanup_free_ struct fiemap *fiemap = NULL;
         _cleanup_close_ int fd = -1;
@@ -84,6 +91,7 @@ int main(int argc, char* argv[]) {
         if (getuid() != 0)
                 log_warning("This program is unlikely to work for unprivileged users");
 
+        test_parse_sleep_config();
         test_sleep();
 
         if (argc <= 1)

src/test/test-strbuf.c

@@ -18,7 +18,7 @@ static ssize_t add_string(struct strbuf *sb, const char *s) {
 }
 
 static void test_strbuf(void) {
-        struct strbuf *sb;
+        _cleanup_(strbuf_cleanupp) struct strbuf *sb;
         _cleanup_strv_free_ char **l;
         ssize_t a, b, c, d, e, f, g, h;
 
@@ -72,8 +72,6 @@ static void test_strbuf(void) {
 
         strbuf_complete(sb);
         assert_se(sb->root == NULL);
-
-        strbuf_cleanup(sb);
 }
 
 int main(int argc, const char *argv[]) {

src/udev/udevadm-hwdb.c

@@ -703,7 +703,8 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) {
         if (trie) {
                 if (trie->root)
                         trie_node_cleanup(trie->root);
-                strbuf_cleanup(trie->strings);
+                if (trie->strings)
+                        strbuf_cleanup(trie->strings);
                 free(trie);
         }
         return rc;