From 89de370eddb61949fb3c87abbe544c217079479e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 12:48:25 +0200 Subject: [PATCH 1/7] core/namespace: drop bitfield annotations from boolean fields Such microoptimization makes sense when the structure is used in many many copies, but here's it's not, and the few bytes we save are not worth the extra code the compiler has to generate: return ns_info->mount_apivfs || ns_info->protect_control_groups || ns_info->protect_kernel_tunables || ... before: 49b187: 48 8b 45 f8 mov -0x8(%rbp),%rax 49b18b: 0f b6 00 movzbl (%rax),%eax 49b18e: 83 e0 80 and $0xffffff80,%eax 49b191: 84 c0 test %al,%al 49b193: 75 32 jne 49b1c7 49b195: 48 8b 45 f8 mov -0x8(%rbp),%rax 49b199: 0f b6 00 movzbl (%rax),%eax 49b19c: 83 e0 08 and $0x8,%eax 49b19f: 84 c0 test %al,%al 49b1a1: 75 24 jne 49b1c7 49b1a3: 48 8b 45 f8 mov -0x8(%rbp),%rax 49b1a7: 0f b6 00 movzbl (%rax),%eax 49b1aa: 83 e0 10 and $0x10,%eax 49b1ad: 84 c0 test %al,%al 49b1af: 75 16 jne 49b1c7 after: 49b024: 48 8b 45 f8 mov -0x8(%rbp),%rax 49b028: 0f b6 40 07 movzbl 0x7(%rax),%eax 49b02c: 84 c0 test %al,%al 49b02e: 75 2e jne 49b05e 49b030: 48 8b 45 f8 mov -0x8(%rbp),%rax 49b034: 0f b6 40 03 movzbl 0x3(%rax),%eax 49b038: 84 c0 test %al,%al 49b03a: 75 22 jne 49b05e 49b03c: 48 8b 45 f8 mov -0x8(%rbp),%rax 49b040: 0f b6 40 04 movzbl 0x4(%rax),%eax 49b044: 84 c0 test %al,%al 49b046: 75 16 jne 49b05e --- src/core/namespace.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/core/namespace.h b/src/core/namespace.h index 13cc0e80cb..908e57c42c 100644 --- a/src/core/namespace.h +++ b/src/core/namespace.h @@ -64,15 +64,15 @@ typedef enum ProcSubset { } ProcSubset; struct NamespaceInfo { - bool ignore_protect_paths:1; - bool private_dev:1; - bool private_mounts:1; - bool protect_control_groups:1; - bool protect_kernel_tunables:1; - bool protect_kernel_modules:1; - bool protect_kernel_logs:1; - bool mount_apivfs:1; - bool protect_hostname:1; + bool ignore_protect_paths; + bool private_dev; + bool private_mounts; + bool protect_control_groups; + bool protect_kernel_tunables; + bool protect_kernel_modules; + bool protect_kernel_logs; + bool mount_apivfs; + bool protect_hostname; ProtectHome protect_home; ProtectSystem protect_system; ProtectProc protect_proc; @@ -82,10 +82,10 @@ struct NamespaceInfo { struct BindMount { char *source; char *destination; - bool read_only:1; - bool nosuid:1; - bool recursive:1; - bool ignore_enoent:1; + bool read_only; + bool nosuid; + bool recursive; + bool ignore_enoent; }; struct TemporaryFileSystem { From 4ffd4705fb917a02c814aa4dc85c3389dc95045e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 17:31:08 +0200 Subject: [PATCH 2/7] activate: reduce scope of iterator variable --- src/activate/activate.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index 5d796ab38d..bcc345bf47 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -122,7 +122,6 @@ static int open_sockets(int *epoll_fd, bool accept) { } static int exec_process(const char *name, char **argv, char **env, int start_fd, size_t n_fds) { - _cleanup_strv_free_ char **envp = NULL; _cleanup_free_ char *joined = NULL; size_t n_env = 0, length; @@ -215,15 +214,13 @@ static int exec_process(const char *name, char **argv, char **env, int start_fd, char *e; len = strv_length(arg_fdnames); - if (len == 1) { - size_t i; - - for (i = 1; i < n_fds; i++) { + if (len == 1) + for (size_t i = 1; i < n_fds; i++) { r = strv_extend(&arg_fdnames, arg_fdnames[0]); if (r < 0) - return log_error_errno(r, "Failed to extend strv: %m"); + return log_oom(); } - } else if (len != n_fds) + else if (len != n_fds) log_warning("The number of fd names is different than number of fds: %zu vs %zu", len, n_fds); names = strv_join(arg_fdnames, ":"); From afa8ffae99284ea0c8f146c6e0450e5321bce059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 17:30:47 +0200 Subject: [PATCH 3/7] various: remove assignments of unread variables --- src/activate/activate.c | 1 - src/backlight/backlight.c | 2 +- src/libsystemd-network/sd-radv.c | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index bcc345bf47..bca845ad83 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -199,7 +199,6 @@ static int exec_process(const char *name, char **argv, char **env, int start_fd, return log_error_errno(errno, "Failed to dup connection: %m"); safe_close(start_fd); - start_fd = SD_LISTEN_FDS_START; } if (asprintf((char **) (envp + n_env++), "LISTEN_FDS=%zu", n_fds) < 0) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 682c719f61..9ee2c23683 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -79,7 +79,7 @@ static int find_pci_or_platform_parent(sd_device *device, sd_device **ret) { } else if (streq(subsystem, "pci") && sd_device_get_sysattr_value(parent, "class", &value) >= 0) { - unsigned long class = 0; + unsigned long class; r = safe_atolu(value, &class); if (r < 0) diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index 6163cf1691..66a25c7dd8 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -689,7 +689,6 @@ _public_ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p, int LIST_APPEND(prefix, ra->route_prefixes, p); ra->n_route_prefixes++; - cur = p; if (!dynamic) { log_radv("Added prefix %s/%u", strempty(pretty), p->opt.prefixlen); return 0; From 90e207e41fe19327841988a7efd301833b986b19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 17:32:26 +0200 Subject: [PATCH 4/7] test-bus-chat: add missing return value in check --- src/libsystemd/sd-bus/test-bus-chat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/test-bus-chat.c b/src/libsystemd/sd-bus/test-bus-chat.c index 7a35d912de..f67d06ecec 100644 --- a/src/libsystemd/sd-bus/test-bus-chat.c +++ b/src/libsystemd/sd-bus/test-bus-chat.c @@ -70,7 +70,7 @@ static int server_init(sd_bus **_bus) { goto fail; } - r = sd_bus_get_description(bus, &desc); + assert_se(sd_bus_get_description(bus, &desc) >= 0); assert_se(streq(desc, "my bus!")); log_info("Peer ID is " SD_ID128_FORMAT_STR ".", SD_ID128_FORMAT_VAL(id)); From d52e1c420c6890f2ac2942721d8e0b311715ee4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 17:33:06 +0200 Subject: [PATCH 5/7] sd-{login,netlink,network}: use TAKE_FD() in more places --- src/libsystemd/sd-login/sd-login.c | 13 +++---------- src/libsystemd/sd-netlink/sd-netlink.c | 3 +-- src/libsystemd/sd-network/sd-network.c | 12 +++--------- 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index d8baa6f8af..939b2a5df4 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -997,20 +997,13 @@ _public_ int sd_login_monitor_new(const char *category, sd_login_monitor **m) { if (!good) return -EINVAL; - *m = FD_TO_MONITOR(fd); - fd = -1; - + *m = FD_TO_MONITOR(TAKE_FD(fd)); return 0; } _public_ sd_login_monitor* sd_login_monitor_unref(sd_login_monitor *m) { - int fd; - - if (!m) - return NULL; - - fd = MONITOR_TO_FD(m); - close_nointr(fd); + if (m) + close_nointr(MONITOR_TO_FD(m)); return NULL; } diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index ff0886d40b..6f283e31ff 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -135,8 +135,7 @@ int netlink_open_family(sd_netlink **ret, int family) { r = sd_netlink_open_fd(ret, fd); if (r < 0) return r; - - fd = -1; + TAKE_FD(fd); return 0; } diff --git a/src/libsystemd/sd-network/sd-network.c b/src/libsystemd/sd-network/sd-network.c index ce6ae846c5..832ec1703e 100644 --- a/src/libsystemd/sd-network/sd-network.c +++ b/src/libsystemd/sd-network/sd-network.c @@ -373,19 +373,13 @@ _public_ int sd_network_monitor_new(sd_network_monitor **m, const char *category if (!good) return -EINVAL; - *m = FD_TO_MONITOR(fd); - fd = -1; - + *m = FD_TO_MONITOR(TAKE_FD(fd)); return 0; } _public_ sd_network_monitor* sd_network_monitor_unref(sd_network_monitor *m) { - int fd; - - if (m) { - fd = MONITOR_TO_FD(m); - close_nointr(fd); - } + if (m) + close_nointr(MONITOR_TO_FD(m)); return NULL; } From 0a9bf7fa593682326f1c5c2d7ec3d29a9d59e5cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 17:33:24 +0200 Subject: [PATCH 6/7] logind: use _cleanup_ in one more place --- src/login/logind-dbus.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 9f16b7f565..01ffbb6bad 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -3195,7 +3195,6 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error _cleanup_free_ char *id = NULL; _cleanup_close_ int fifo_fd = -1; Manager *m = userdata; - Inhibitor *i = NULL; InhibitMode mm; InhibitWhat w; pid_t pid; @@ -3278,6 +3277,7 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error } while (hashmap_get(m->inhibitors, id)); + _cleanup_(inhibitor_freep) Inhibitor *i = NULL; r = manager_add_inhibitor(m, id, &i); if (r < 0) return r; @@ -3289,28 +3289,18 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error i->why = strdup(why); i->who = strdup(who); - if (!i->why || !i->who) { - r = -ENOMEM; - goto fail; - } + if (!i->why || !i->who) + return -ENOMEM; fifo_fd = inhibitor_create_fifo(i); - if (fifo_fd < 0) { - r = fifo_fd; - goto fail; - } + if (fifo_fd < 0) + return fifo_fd; r = inhibitor_start(i); if (r < 0) - goto fail; + return r; return sd_bus_reply_method_return(message, "h", fifo_fd); - -fail: - if (i) - inhibitor_free(i); - - return r; } static const sd_bus_vtable manager_vtable[] = { From fec5929f8b0479be780dd730f276b8c9738ac48e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 13:05:31 +0200 Subject: [PATCH 7/7] shared/conf-parser: drop redundant cast to boolean parse_boolean returns either 0 or 1 or error, and we checked for errors earlier already. --- src/shared/conf-parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index 90f08252ab..02a27e3a88 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -703,7 +703,7 @@ int config_parse_tristate( return 0; } - *t = !!k; + *t = k; return 0; }