From 7e92ab4eb83bab01594867424161d1e9397a5ce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 23 Apr 2021 14:20:14 +0200 Subject: [PATCH 01/10] meson: make one check shorter --- src/boot/efi/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/efi/meson.build b/src/boot/efi/meson.build index 574feedb98d..47768931082 100644 --- a/src/boot/efi/meson.build +++ b/src/boot/efi/meson.build @@ -190,7 +190,7 @@ if have_gnu_efi '-znocombreloc', '-L', efi_libdir, efi_crt0] - if efi_arch == 'aarch64' or efi_arch == 'arm' or efi_arch == 'riscv64' + if ['aarch64', 'arm', 'riscv64'].contains(efi_arch) # Aarch64, ARM32 and 64bit RISC-V don't have an EFI capable objcopy. # Use 'binary' instead, and add required symbols manually. efi_ldflags += ['--defsym=EFI_SUBSYSTEM=0xa'] From 431733b866f184527649996193874885ac059b70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 21 Feb 2021 21:29:00 +0100 Subject: [PATCH 02/10] core: minor simplification --- src/core/main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 2f36d752bca..12948ca964c 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2549,18 +2549,16 @@ static void setup_console_terminal(bool skip_setup) { static bool early_skip_setup_check(int argc, char *argv[]) { bool found_deserialize = false; - int i; /* Determine if this is a reexecution or normal bootup. We do the full command line parsing much later, so * let's just have a quick peek here. Note that if we have switched root, do all the special setup things * anyway, even if in that case we also do deserialization. */ - for (i = 1; i < argc; i++) { + for (int i = 1; i < argc; i++) if (streq(argv[i], "--switched-root")) return false; /* If we switched root, don't skip the setup. */ else if (streq(argv[i], "--deserialize")) found_deserialize = true; - } return found_deserialize; /* When we are deserializing, then we are reexecuting, hence avoid the extensive setup */ } From 806a93623c1d20974ad356bcb7fb46d32d5bb037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 7 Apr 2021 10:29:39 +0200 Subject: [PATCH 03/10] core/cgroup: use assert_se() more --- src/core/cgroup.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 9fbf64b7bee..6f9e68f650e 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -305,8 +305,7 @@ static int unit_compare_memory_limit(Unit *u, const char *property_name, uint64_ if (!FLAGS_SET(m, CGROUP_MASK_MEMORY)) return -ENODATA; - c = unit_get_cgroup_context(u); - assert(c); + assert_se(c = unit_get_cgroup_context(u)); if (streq(property_name, "MemoryLow")) { unit_value = unit_get_ancestor_memory_low(u); @@ -407,8 +406,7 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { assert(u); assert(f); - c = unit_get_cgroup_context(u); - assert(c); + assert_se(c = unit_get_cgroup_context(u)); prefix = strempty(prefix); @@ -1071,8 +1069,7 @@ static bool unit_has_unified_memory_config(Unit *u) { assert(u); - c = unit_get_cgroup_context(u); - assert(c); + assert_se(c = unit_get_cgroup_context(u)); return unit_get_ancestor_memory_min(u) > 0 || unit_get_ancestor_memory_low(u) > 0 || c->memory_high != CGROUP_LIMIT_MAX || c->memory_max != CGROUP_LIMIT_MAX || @@ -1597,9 +1594,7 @@ static CGroupMask unit_get_cgroup_mask(Unit *u) { assert(u); - c = unit_get_cgroup_context(u); - - assert(c); + assert_se(c = unit_get_cgroup_context(u)); /* Figure out which controllers we need, based on the cgroup context object */ From 0f6bb1ed05ceba91d8529c74379613ec9ebf50dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 7 Apr 2021 10:52:30 +0200 Subject: [PATCH 04/10] core: remove spurious % in dump string --- src/core/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 6f9e68f650e..5402fffacf8 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -449,7 +449,7 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { "%sManagedOOMSwap: %s\n" "%sManagedOOMMemoryPressure: %s\n" "%sManagedOOMMemoryPressureLimit: " PERMYRIAD_AS_PERCENT_FORMAT_STR "\n" - "%sManagedOOMPreference: %s%%\n", + "%sManagedOOMPreference: %s\n", prefix, yes_no(c->cpu_accounting), prefix, yes_no(c->io_accounting), prefix, yes_no(c->blockio_accounting), From 32f65484d70e6f749534d176022b1e4760f22ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Apr 2021 23:30:26 +0200 Subject: [PATCH 05/10] basic/io-util: invert return value from IOVEC_INCREMENT() We would try to return a value that could be nonzero only if the kernel reported writing more bytes than we gave to it, hopefully a rare occurence. Instead, assert that this doesn't happen. Instead, return true if we got to the end of the iovec array. The caller can use this information to know that the whole iovec array was written. This allows one loop to be dropped in write_to_syslog(). Also drop _unlikely_: this function is called with very short arrays, and it *is* likely that we trigger this condition. Let's just let the compiler generate normal code without giving it a potentially false hint. --- src/basic/io-util.h | 17 +++++++++++------ src/basic/log.c | 6 +++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/basic/io-util.h b/src/basic/io-util.h index d98817f7609..d4bb14fba43 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -33,14 +33,17 @@ static inline size_t IOVEC_TOTAL_SIZE(const struct iovec *i, size_t n) { return r; } -static inline size_t IOVEC_INCREMENT(struct iovec *i, size_t n, size_t k) { - size_t j; +static inline bool IOVEC_INCREMENT(struct iovec *i, size_t n, size_t k) { + /* Returns true if there is nothing else to send (bytes written cover all of the iovec), + * false if there's still work to do. */ - for (j = 0; j < n; j++) { + for (size_t j = 0; j < n; j++) { size_t sub; - if (_unlikely_(k <= 0)) - break; + if (i[j].iov_len == 0) + continue; + if (k == 0) + return false; sub = MIN(i[j].iov_len, k); i[j].iov_len -= sub; @@ -48,7 +51,9 @@ static inline size_t IOVEC_INCREMENT(struct iovec *i, size_t n, size_t k) { k -= sub; } - return k; + assert(k == 0); /* Anything else would mean that we wrote more bytes than available, + * or the kernel reported writing more bytes than sent. */ + return true; } static inline bool FILE_SIZE_VALID(uint64_t l) { diff --git a/src/basic/log.c b/src/basic/log.c index 595db0c395a..c098939ed1c 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -491,11 +491,11 @@ static int write_to_syslog( if (n < 0) return -errno; - if (!syslog_is_stream || - (size_t) n >= IOVEC_TOTAL_SIZE(iovec, ELEMENTSOF(iovec))) + if (!syslog_is_stream) break; - IOVEC_INCREMENT(iovec, ELEMENTSOF(iovec), n); + if (IOVEC_INCREMENT(iovec, ELEMENTSOF(iovec), n)) + break; } return 1; From cf1e6e626056817a31b022a2490e84f15cb5dfa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 4 May 2021 14:49:47 +0200 Subject: [PATCH 06/10] resolved: directly initialize iovec array --- src/resolve/resolved-dns-stream.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c index 10641f6ac59..3e6505cd75b 100644 --- a/src/resolve/resolved-dns-stream.c +++ b/src/resolve/resolved-dns-stream.c @@ -315,15 +315,14 @@ static int on_stream_io(sd_event_source *es, int fd, uint32_t revents, void *use s->write_packet && s->n_written < sizeof(s->write_size) + s->write_packet->size) { - struct iovec iov[2]; - ssize_t ss; + struct iovec iov[] = { + IOVEC_MAKE(&s->write_size, sizeof(s->write_size)), + IOVEC_MAKE(DNS_PACKET_DATA(s->write_packet), s->write_packet->size), + }; - iov[0] = IOVEC_MAKE(&s->write_size, sizeof(s->write_size)); - iov[1] = IOVEC_MAKE(DNS_PACKET_DATA(s->write_packet), s->write_packet->size); + IOVEC_INCREMENT(iov, ELEMENTSOF(iov), s->n_written); - IOVEC_INCREMENT(iov, 2, s->n_written); - - ss = dns_stream_writev(s, iov, 2, 0); + ssize_t ss = dns_stream_writev(s, iov, ELEMENTSOF(iov), 0); if (ss < 0) { if (!IN_SET(-ss, EINTR, EAGAIN)) return dns_stream_complete(s, -ss); From 2f9d1da95a1ad34a56a29953d7b5f7051590cf8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Apr 2021 23:30:45 +0200 Subject: [PATCH 07/10] basic/io-util: move iterator variable declarations into loop headers --- src/basic/io-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/io-util.h b/src/basic/io-util.h index d4bb14fba43..ce54ca228bf 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -25,9 +25,9 @@ int fd_wait_for_event(int fd, int event, usec_t timeout); ssize_t sparse_write(int fd, const void *p, size_t sz, size_t run_length); static inline size_t IOVEC_TOTAL_SIZE(const struct iovec *i, size_t n) { - size_t j, r = 0; + size_t r = 0; - for (j = 0; j < n; j++) + for (size_t j = 0; j < n; j++) r += i[j].iov_len; return r; From 905a0abd22d4eb43547454e22ca36afe93878ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Apr 2021 23:52:10 +0200 Subject: [PATCH 08/10] basic/log: directly initialize iovec arrays It doesn't change much, but removes a bunch of explicit size and index handling and unnecessary initializations. --- src/basic/log.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index c098939ed1c..b4c16a3e1b9 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -452,11 +452,6 @@ static int write_to_syslog( char header_priority[2 + DECIMAL_STR_MAX(int) + 1], header_time[64], header_pid[4 + DECIMAL_STR_MAX(pid_t) + 1]; - struct iovec iovec[5] = {}; - struct msghdr msghdr = { - .msg_iov = iovec, - .msg_iovlen = ELEMENTSOF(iovec), - }; time_t t; struct tm tm; @@ -474,15 +469,21 @@ static int write_to_syslog( xsprintf(header_pid, "["PID_FMT"]: ", getpid_cached()); - iovec[0] = IOVEC_MAKE_STRING(header_priority); - iovec[1] = IOVEC_MAKE_STRING(header_time); - iovec[2] = IOVEC_MAKE_STRING(program_invocation_short_name); - iovec[3] = IOVEC_MAKE_STRING(header_pid); - iovec[4] = IOVEC_MAKE_STRING(buffer); + struct iovec iovec[] = { + IOVEC_MAKE_STRING(header_priority), + IOVEC_MAKE_STRING(header_time), + IOVEC_MAKE_STRING(program_invocation_short_name), + IOVEC_MAKE_STRING(header_pid), + IOVEC_MAKE_STRING(buffer), + }; + struct msghdr msghdr = { + .msg_iov = iovec, + .msg_iovlen = ELEMENTSOF(iovec), + }; /* When using syslog via SOCK_STREAM separate the messages by NUL chars */ if (syslog_is_stream) - iovec[4].iov_len++; + iovec[ELEMENTSOF(iovec) - 1].iov_len++; for (;;) { ssize_t n; @@ -520,7 +521,6 @@ static int write_to_kmsg( char header_priority[2 + DECIMAL_STR_MAX(int) + 1], header_pid[4 + DECIMAL_STR_MAX(pid_t) + 1]; - struct iovec iovec[5] = {}; if (kmsg_fd < 0) return 0; @@ -531,11 +531,13 @@ static int write_to_kmsg( xsprintf(header_priority, "<%i>", level); xsprintf(header_pid, "["PID_FMT"]: ", getpid_cached()); - iovec[0] = IOVEC_MAKE_STRING(header_priority); - iovec[1] = IOVEC_MAKE_STRING(program_invocation_short_name); - iovec[2] = IOVEC_MAKE_STRING(header_pid); - iovec[3] = IOVEC_MAKE_STRING(buffer); - iovec[4] = IOVEC_MAKE_STRING("\n"); + const struct iovec iovec[] = { + IOVEC_MAKE_STRING(header_priority), + IOVEC_MAKE_STRING(program_invocation_short_name), + IOVEC_MAKE_STRING(header_pid), + IOVEC_MAKE_STRING(buffer), + IOVEC_MAKE_STRING("\n"), + }; if (writev(kmsg_fd, iovec, ELEMENTSOF(iovec)) < 0) return -errno; @@ -959,7 +961,7 @@ int log_struct_internal( if (journal_fd >= 0) { char header[LINE_MAX]; struct iovec iovec[17] = {}; - size_t n = 0, i; + size_t n = 0; int r; struct msghdr mh = { .msg_iov = iovec, @@ -981,7 +983,7 @@ int log_struct_internal( } va_end(ap); - for (i = 1; i < n; i += 2) + for (size_t i = 1; i < n; i += 2) free(iovec[i].iov_base); if (!fallback) { From 4c1f9343b2c7f119986e9a1b7d55d537417a808c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Apr 2021 23:53:07 +0200 Subject: [PATCH 09/10] core/cgroup: remove some unnecessary variables --- src/core/cgroup.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 5402fffacf8..a44cf9368c7 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1026,17 +1026,14 @@ static void cgroup_apply_io_device_latency(Unit *u, const char *dev_path, usec_t } static void cgroup_apply_io_device_limit(Unit *u, const char *dev_path, uint64_t *limits) { - char limit_bufs[_CGROUP_IO_LIMIT_TYPE_MAX][DECIMAL_STR_MAX(uint64_t)]; - char buf[DECIMAL_STR_MAX(dev_t)*2+2+(6+DECIMAL_STR_MAX(uint64_t)+1)*4]; - CGroupIOLimitType type; + char limit_bufs[_CGROUP_IO_LIMIT_TYPE_MAX][DECIMAL_STR_MAX(uint64_t)], + buf[DECIMAL_STR_MAX(dev_t)*2+2+(6+DECIMAL_STR_MAX(uint64_t)+1)*4]; dev_t dev; - int r; - r = lookup_block_device(dev_path, &dev); - if (r < 0) + if (lookup_block_device(dev_path, &dev) < 0) return; - for (type = 0; type < _CGROUP_IO_LIMIT_TYPE_MAX; type++) + for (CGroupIOLimitType type = 0; type < _CGROUP_IO_LIMIT_TYPE_MAX; type++) if (limits[type] != cgroup_io_limit_defaults[type]) xsprintf(limit_bufs[type], "%" PRIu64, limits[type]); else @@ -1051,10 +1048,8 @@ static void cgroup_apply_io_device_limit(Unit *u, const char *dev_path, uint64_t static void cgroup_apply_blkio_device_limit(Unit *u, const char *dev_path, uint64_t rbps, uint64_t wbps) { char buf[DECIMAL_STR_MAX(dev_t)*2+2+DECIMAL_STR_MAX(uint64_t)+1]; dev_t dev; - int r; - r = lookup_block_device(dev_path, &dev); - if (r < 0) + if (lookup_block_device(dev_path, &dev) < 0) return; sprintf(buf, "%u:%u %" PRIu64 "\n", major(dev), minor(dev), rbps); From edb83bceec11986ce7a50174a7b4798d0e2383b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Apr 2021 23:54:32 +0200 Subject: [PATCH 10/10] core/automount: drop unecessary cast --- src/core/automount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/automount.c b/src/core/automount.c index f0fa5c8ca98..dc92f9c0e44 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -651,7 +651,7 @@ fail: static void *expire_thread(void *p) { struct autofs_dev_ioctl param; - _cleanup_(expire_data_freep) struct expire_data *data = (struct expire_data*)p; + _cleanup_(expire_data_freep) struct expire_data *data = p; int r; assert(data->dev_autofs_fd >= 0);