diff --git a/src/basic/unit-file.c b/src/basic/unit-file.c index 83c29bb25f..ac0236d635 100644 --- a/src/basic/unit-file.c +++ b/src/basic/unit-file.c @@ -689,6 +689,7 @@ static int add_names( return log_debug_errno(r, "Cannot find instance fragment %s: %m", inst); if (inst_fragment && + fragment_basename && !streq(basename(inst_fragment), fragment_basename)) { log_debug("Instance %s has fragment %s and is not an alias of %s.", inst, inst_fragment, unit_name); diff --git a/src/libsystemd/sd-bus/test-bus-address.c b/src/libsystemd/sd-bus/test-bus-address.c index ec9ee83fac..347ba1a39b 100644 --- a/src/libsystemd/sd-bus/test-bus-address.c +++ b/src/libsystemd/sd-bus/test-bus-address.c @@ -15,11 +15,9 @@ static void test_one_address(sd_bus *b, r = bus_set_address_system_remote(b, host); log_info("\"%s\" → %d, \"%s\"", host, r, strna(r >= 0 ? b->address : NULL)); - if (result < 0 || expected) { - assert_se(r == result); - if (r >= 0) - assert_se(streq(b->address, expected)); - } + assert_se(r == result); + if (r >= 0) + assert_se(streq_ptr(b->address, expected)); } TEST(bus_set_address_system_remote) { @@ -38,8 +36,8 @@ TEST(bus_set_address_system_remote) { 0, "unixexec:path=ssh,argv1=-xT,argv2=-p,argv3=123,argv4=--,argv5=host,argv6=systemd-stdio-bridge"); test_one_address(b, "host:123:123", -EINVAL, NULL); - test_one_address(b, "host:", - -EINVAL, NULL); + test_one_address(b, "host:", + -EINVAL, NULL); test_one_address(b, "user@host", 0, "unixexec:path=ssh,argv1=-xT,argv2=--,argv3=user%40host,argv4=systemd-stdio-bridge"); test_one_address(b, "user@host@host", diff --git a/src/libsystemd/sd-hwdb/sd-hwdb.c b/src/libsystemd/sd-hwdb/sd-hwdb.c index f73d1fc21e..51be54b1e1 100644 --- a/src/libsystemd/sd-hwdb/sd-hwdb.c +++ b/src/libsystemd/sd-hwdb/sd-hwdb.c @@ -283,7 +283,6 @@ static int trie_search_f(sd_hwdb *hwdb, const char *search) { static int hwdb_new(const char *path, sd_hwdb **ret) { _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb = NULL; - const char *hwdb_bin_path = NULL; const char sig[] = HWDB_SIG; assert_return(ret, -EINVAL); @@ -294,20 +293,20 @@ static int hwdb_new(const char *path, sd_hwdb **ret) { hwdb->n_ref = 1; - /* find hwdb.bin in hwdb_bin_paths, or from an explicit path if provided */ + /* Find hwdb.bin in the explicit path if provided, or iterate over hwdb_bin_paths otherwise */ if (!isempty(path)) { log_debug("Trying to open \"%s\"...", path); hwdb->f = fopen(path, "re"); if (!hwdb->f) return log_debug_errno(errno, "Failed to open %s: %m", path); } else { - NULSTR_FOREACH(hwdb_bin_path, hwdb_bin_paths) { - log_debug("Trying to open \"%s\"...", hwdb_bin_path); - hwdb->f = fopen(hwdb_bin_path, "re"); + NULSTR_FOREACH(path, hwdb_bin_paths) { + log_debug("Trying to open \"%s\"...", path); + hwdb->f = fopen(path, "re"); if (hwdb->f) break; if (errno != ENOENT) - return log_debug_errno(errno, "Failed to open %s: %m", hwdb_bin_path); + return log_debug_errno(errno, "Failed to open %s: %m", path); } if (!hwdb->f) @@ -316,23 +315,20 @@ static int hwdb_new(const char *path, sd_hwdb **ret) { } if (fstat(fileno(hwdb->f), &hwdb->st) < 0) - return log_debug_errno(errno, "Failed to stat %s: %m", hwdb_bin_path); + return log_debug_errno(errno, "Failed to stat %s: %m", path); if (hwdb->st.st_size < (off_t) offsetof(struct trie_header_f, strings_len) + 8) - return log_debug_errno(SYNTHETIC_ERRNO(EIO), - "File %s is too short: %m", hwdb_bin_path); + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "File %s is too short: %m", path); if (file_offset_beyond_memory_size(hwdb->st.st_size)) - return log_debug_errno(SYNTHETIC_ERRNO(EFBIG), - "File %s is too long: %m", hwdb_bin_path); + return log_debug_errno(SYNTHETIC_ERRNO(EFBIG), "File %s is too long: %m", path); hwdb->map = mmap(0, hwdb->st.st_size, PROT_READ, MAP_SHARED, fileno(hwdb->f), 0); if (hwdb->map == MAP_FAILED) - return log_debug_errno(errno, "Failed to map %s: %m", hwdb_bin_path); + return log_debug_errno(errno, "Failed to map %s: %m", path); if (memcmp(hwdb->map, sig, sizeof(hwdb->head->signature)) != 0 || (size_t) hwdb->st.st_size != le64toh(hwdb->head->file_size)) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), - "Failed to recognize the format of %s", - hwdb_bin_path); + "Failed to recognize the format of %s", path); log_debug("=== trie on-disk ==="); log_debug("tool version: %"PRIu64, le64toh(hwdb->head->tool_version)); diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 705a35de3f..5138993f05 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -147,8 +147,6 @@ static void set_location(sd_journal *j, JournalFile *f, Object *o) { } static int match_is_valid(const void *data, size_t size) { - const char *b, *p; - assert(data); if (size < 2) @@ -157,8 +155,8 @@ static int match_is_valid(const void *data, size_t size) { if (((char*) data)[0] == '_' && ((char*) data)[1] == '_') return false; - b = data; - for (p = b; p < b + size; p++) { + const char *b = data; + for (const char *p = b; p < b + size; p++) { if (*p == '=') return p > b; @@ -180,9 +178,8 @@ static int match_is_valid(const void *data, size_t size) { static bool same_field(const void *_a, size_t s, const void *_b, size_t t) { const uint8_t *a = _a, *b = _b; - size_t j; - for (j = 0; j < s && j < t; j++) { + for (size_t j = 0; j < s && j < t; j++) { if (a[j] != b[j]) return false; @@ -818,7 +815,7 @@ static int next_beyond_location(sd_journal *j, JournalFile *f, direction_t direc static int real_journal_next(sd_journal *j, direction_t direction) { JournalFile *new_file = NULL; - unsigned i, n_files; + unsigned n_files; const void **files; Object *o; int r; @@ -830,7 +827,7 @@ static int real_journal_next(sd_journal *j, direction_t direction) { if (r < 0) return r; - for (i = 0; i < n_files; i++) { + for (unsigned i = 0; i < n_files; i++) { JournalFile *f = (JournalFile *)files[i]; bool found; @@ -1268,89 +1265,92 @@ static int add_any_file( int fd, const char *path) { - bool close_fd = false; + _cleanup_close_ int our_fd = -1; JournalFile *f; struct stat st; - int r, k; + int r; assert(j); assert(fd >= 0 || path); if (fd < 0) { + assert(path); /* For gcc. */ if (j->toplevel_fd >= 0) /* If there's a top-level fd defined make the path relative, explicitly, since otherwise * openat() ignores the first argument. */ - fd = openat(j->toplevel_fd, skip_slash(path), O_RDONLY|O_CLOEXEC|O_NONBLOCK); + fd = our_fd = openat(j->toplevel_fd, skip_slash(path), O_RDONLY|O_CLOEXEC|O_NONBLOCK); else - fd = open(path, O_RDONLY|O_CLOEXEC|O_NONBLOCK); + fd = our_fd = open(path, O_RDONLY|O_CLOEXEC|O_NONBLOCK); if (fd < 0) { r = log_debug_errno(errno, "Failed to open journal file %s: %m", path); - goto finish; + goto error; } - close_fd = true; - r = fd_nonblock(fd, false); if (r < 0) { r = log_debug_errno(errno, "Failed to turn off O_NONBLOCK for %s: %m", path); - goto finish; + goto error; } } if (fstat(fd, &st) < 0) { - r = log_debug_errno(errno, "Failed to fstat file '%s': %m", path); - goto finish; + r = log_debug_errno(errno, "Failed to fstat %s: %m", path ?: "fd"); + goto error; } r = stat_verify_regular(&st); if (r < 0) { - log_debug_errno(r, "Refusing to open '%s', as it is not a regular file.", path); - goto finish; + log_debug_errno(r, "Refusing to open %s: %m", path ?: "fd"); + goto error; } - f = ordered_hashmap_get(j->files, path); - if (f) { - if (stat_inode_same(&f->last_stat, &st)) { + if (path) { + f = ordered_hashmap_get(j->files, path); + if (f) { + if (stat_inode_same(&f->last_stat, &st)) { + /* We already track this file, under the same path and with the same + * device/inode numbers, it's hence really the same. Mark this file as seen + * in this generation. This is used to GC old files in process_q_overflow() + * to detect journal files that are still there and discern them from those + * which are gone. */ - /* We already track this file, under the same path and with the same device/inode numbers, it's - * hence really the same. Mark this file as seen in this generation. This is used to GC old - * files in process_q_overflow() to detect journal files that are still there and discern them - * from those which are gone. */ + f->last_seen_generation = j->generation; + return 0; + } - f->last_seen_generation = j->generation; - r = 0; - goto finish; + /* So we tracked a file under this name, but it has a different inode/device. In that + * case, it got replaced (probably due to rotation?), let's drop it hence from our + * list. */ + remove_file_real(j, f); + f = NULL; } - - /* So we tracked a file under this name, but it has a different inode/device. In that case, it got - * replaced (probably due to rotation?), let's drop it hence from our list. */ - remove_file_real(j, f); - f = NULL; } if (ordered_hashmap_size(j->files) >= JOURNAL_FILES_MAX) { - log_debug("Too many open journal files, not adding %s.", path); - r = -ETOOMANYREFS; - goto finish; + r = log_debug_errno(SYNTHETIC_ERRNO(ETOOMANYREFS), + "Too many open journal files, not adding %s.", path ?: "fd"); + goto error; } r = journal_file_open(fd, path, O_RDONLY, 0, 0, 0, NULL, j->mmap, NULL, &f); if (r < 0) { - log_debug_errno(r, "Failed to open journal file %s: %m", path); - goto finish; + log_debug_errno(r, "Failed to open journal file %s: %m", path ?: "from fd"); + goto error; } /* journal_file_dump(f); */ + /* journal_file_open() generates an replacement fname if necessary, so we can use f->path. */ r = ordered_hashmap_put(j->files, f->path, f); if (r < 0) { - f->close_fd = false; /* make sure journal_file_close() doesn't close the caller's fd (or our own). We'll let the caller do that, or ourselves */ + f->close_fd = false; /* Make sure journal_file_close() doesn't close the caller's fd + * (or our own). The caller or we will do that ourselves. */ (void) journal_file_close(f); - goto finish; + goto error; } - close_fd = false; /* the fd is now owned by the JournalFile object */ + TAKE_FD(our_fd); /* the fd is now owned by the JournalFile object */ f->last_seen_generation = j->generation; @@ -1361,18 +1361,10 @@ static int add_any_file( log_debug("File %s added.", f->path); - r = 0; - -finish: - if (close_fd) - safe_close(fd); - - if (r < 0) { - k = journal_put_error(j, r, path); - if (k < 0) - return k; - } + return 0; +error: + (void) journal_put_error(j, r, path); /* path==NULL is OK. */ return r; } @@ -2089,7 +2081,6 @@ _public_ int sd_journal_open_directory_fd(sd_journal **ret, int fd, int flags) { _public_ int sd_journal_open_files_fd(sd_journal **ret, int fds[], unsigned n_fds, int flags) { JournalFile *f; _cleanup_(sd_journal_closep) sd_journal *j = NULL; - unsigned i; int r; assert_return(ret, -EINVAL); @@ -2100,7 +2091,7 @@ _public_ int sd_journal_open_files_fd(sd_journal **ret, int fds[], unsigned n_fd if (!j) return -ENOMEM; - for (i = 0; i < n_fds; i++) { + for (unsigned i = 0; i < n_fds; i++) { struct stat st; if (fds[i] < 0) { @@ -2237,8 +2228,6 @@ _public_ int sd_journal_get_monotonic_usec(sd_journal *j, uint64_t *ret, sd_id12 } static bool field_is_valid(const char *field) { - const char *p; - assert(field); if (isempty(field)) @@ -2247,7 +2236,7 @@ static bool field_is_valid(const char *field) { if (startswith(field, "__")) return false; - for (p = field; *p; p++) { + for (const char *p = field; *p; p++) { if (*p == '_') continue; @@ -2266,7 +2255,6 @@ static bool field_is_valid(const char *field) { _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void **data, size_t *size) { JournalFile *f; - uint64_t i, n; size_t field_length; int r; Object *o; @@ -2291,8 +2279,8 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** field_length = strlen(field); - n = journal_file_entry_n_items(o); - for (i = 0; i < n; i++) { + uint64_t n = journal_file_entry_n_items(o); + for (uint64_t i = 0; i < n; i++) { Object *d; uint64_t p, l; size_t t; diff --git a/src/test/test-load-fragment.c b/src/test/test-load-fragment.c index 9cbf22312f..cc6301e9dc 100644 --- a/src/test/test-load-fragment.c +++ b/src/test/test-load-fragment.c @@ -70,10 +70,10 @@ static void check_execcommand(ExecCommand *c, assert_se(c); log_info("expect: \"%s\" [\"%s\" \"%s\" \"%s\"]", - path, argv0 ?: path, argv1, argv2); + path, argv0 ?: path, strnull(argv1), strnull(argv2)); n = strv_length(c->argv); log_info("actual: \"%s\" [\"%s\" \"%s\" \"%s\"]", - c->path, c->argv[0], n > 0 ? c->argv[1] : NULL, n > 1 ? c->argv[2] : NULL); + c->path, c->argv[0], n > 0 ? c->argv[1] : "(null)", n > 1 ? c->argv[2] : "(null)"); assert_se(streq(c->path, path)); assert_se(streq(c->argv[0], argv0 ?: path)); if (n > 0) diff --git a/src/test/test-mountpoint-util.c b/src/test/test-mountpoint-util.c index 6f9e68f4d9..6daab022aa 100644 --- a/src/test/test-mountpoint-util.c +++ b/src/test/test-mountpoint-util.c @@ -20,7 +20,7 @@ static void test_mount_propagation_flags_one(const char *name, int ret, unsigned long expected) { unsigned long flags; - log_info("/* %s(%s) */", __func__, name); + log_info("/* %s(%s) */", __func__, strnull(name)); assert_se(mount_propagation_flags_from_string(name, &flags) == ret);