From fb01ab09d3499b1a2e8aac42ec11213b71d6dd6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 11:33:35 +0100 Subject: [PATCH 01/25] TODO: drop entry about renaming NamespaceId is inconsistent with our general spelling rules which say that capitalization of acronyms should not be changed when they are concatenated in CapitalizedCamelCase strings. But NamespaceId was added in f2ef8b28a5266a0070b0cb8b6375e418a5300975, before v249, so it's too late to change it. OTOH, NamespaceNSID cannot really be spelled in any other way. "NamespaceNsid" would be quite bad. So let's just ignore the inconsistency in naming, this seems to be the least bad option. --- TODO | 1 - 1 file changed, 1 deletion(-) diff --git a/TODO b/TODO index 3b76e98bde..7264c573b6 100644 --- a/TODO +++ b/TODO @@ -2550,7 +2550,6 @@ Features: - allow Name= to be specified repeatedly in the [Match] section. Maybe also support Name=foo*|bar*|baz ? - whenever uplink info changes, make DHCP server send out FORCERENEW - - figure out spelling: NamespaceId vs. NamespaceNSID * in networkd, when matching device types, fix up DEVTYPE rubbish the kernel passes to us From 892c5902ae11e5b0ca0494411067794d763baf45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 Mar 2024 13:16:51 +0100 Subject: [PATCH 02/25] src/basic: add yet another strdup helper It's a bit ugly to have both strdup_to() and strdup_to_full(). I initially started with one variant, but then in some functions we want the additional info, while in many other places, having 1 instead of 0 causes the return value of whole chains of functions to be changed. It *probably* wouldn't cause any difference, but there is at least of bunch of tests that would need to be updated, so in the end it seems to have the two variants. The output param is first to match free_and_strdup() and other similar functions. --- src/basic/string-util.c | 18 ++++++++++++++++++ src/basic/string-util.h | 6 ++++++ src/test/test-string-util.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index c1e7e6e622..6e8d10747b 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1126,6 +1126,24 @@ int free_and_strndup(char **p, const char *s, size_t l) { return 1; } +int strdup_to_full(char **ret, const char *src) { + if (!src) { + if (ret) + *ret = NULL; + + return 0; + } else { + if (ret) { + char *t = strdup(src); + if (!t) + return -ENOMEM; + *ret = t; + } + + return 1; + } +}; + bool string_is_safe(const char *p) { if (!p) return false; diff --git a/src/basic/string-util.h b/src/basic/string-util.h index e162765aa7..c402cd3af0 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -224,6 +224,12 @@ static inline int free_and_strdup_warn(char **p, const char *s) { } int free_and_strndup(char **p, const char *s, size_t l); +int strdup_to_full(char **ret, const char *src); +static inline int strdup_to(char **ret, const char *src) { + int r = strdup_to_full(ASSERT_PTR(ret), src); + return r < 0 ? r : 0; /* Suppress return value of 1. */ +} + bool string_is_safe(const char *p) _pure_; DISABLE_WARNING_STRINGOP_TRUNCATION; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 92f1083a4c..3b003e885f 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -88,6 +88,35 @@ TEST(free_and_strndup) { } } +TEST(strdup_to_full) { + _cleanup_free_ char *dst; + + assert_se(strdup_to_full(NULL, NULL) == 0); + assert_se(strdup_to_full(&dst, NULL) == 0); + + assert_se(strdup_to_full(NULL, "") == 1); + assert_se(strdup_to_full(&dst, "") == 1); + assert_se(streq_ptr(dst, "")); + dst = mfree(dst); + + assert_se(strdup_to_full(NULL, "x") == 1); + assert_se(strdup_to_full(&dst, "x") == 1); + assert_se(streq_ptr(dst, "x")); +} + +TEST(strdup_to) { + _cleanup_free_ char *dst; + + assert_se(strdup_to(&dst, NULL) == 0); + + assert_se(strdup_to(&dst, "") == 0); + assert_se(streq_ptr(dst, "")); + dst = mfree(dst); + + assert_se(strdup_to(&dst, "x") == 0); + assert_se(streq_ptr(dst, "x")); +} + TEST(ascii_strcasecmp_n) { assert_se(ascii_strcasecmp_n("", "", 0) == 0); assert_se(ascii_strcasecmp_n("", "", 1) == 0); From d3a3d2d328af63dce207146c58df72911f6fd8a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 Mar 2024 13:20:15 +0100 Subject: [PATCH 03/25] hostnamed: use strdup_to_full() --- src/hostname/hostnamed.c | 31 ++----------------------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c index 83a988154f..82d08803fa 100644 --- a/src/hostname/hostnamed.c +++ b/src/hostname/hostnamed.c @@ -211,7 +211,6 @@ static bool use_dmi_data(void) { static int get_dmi_data(const char *database_key, const char *regular_key, char **ret) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; - _cleanup_free_ char *b = NULL; const char *s = NULL; int r; @@ -227,17 +226,7 @@ static int get_dmi_data(const char *database_key, const char *regular_key, char if (!s && regular_key) (void) sd_device_get_property_value(device, regular_key, &s); - if (!ret) - return !!s; - - if (s) { - b = strdup(s); - if (!b) - return -ENOMEM; - } - - *ret = TAKE_PTR(b); - return !!s; + return strdup_to_full(ret, s); } static int get_hardware_vendor(char **ret) { @@ -264,23 +253,7 @@ static int get_hardware_firmware_data(const char *sysattr, char **ret) { (void) sd_device_get_sysattr_value(device, sysattr, &s); - bool empty = isempty(s); - - if (ret) { - if (empty) - *ret = NULL; - else { - _cleanup_free_ char *b = NULL; - - b = strdup(s); - if (!b) - return -ENOMEM; - - *ret = TAKE_PTR(b); - } - } - - return !empty; + return strdup_to_full(ret, empty_to_null(s)); } static int get_hardware_serial(char **ret) { From 8922a728f732a716ecd17dd67cd39bc1a0fc4aa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 Mar 2024 13:39:50 +0100 Subject: [PATCH 04/25] basic/cgroup-util: use strdup_to() and strdup_to_full() as appropriate --- src/basic/cgroup-util.c | 94 +++++++++-------------------------------- 1 file changed, 21 insertions(+), 73 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 68b1584629..60b4e3f226 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -135,7 +135,7 @@ int cg_read_event( return r; for (const char *p = content;;) { - _cleanup_free_ char *line = NULL, *key = NULL, *val = NULL; + _cleanup_free_ char *line = NULL, *key = NULL; const char *q; r = extract_first_word(&p, &line, "\n", 0); @@ -154,12 +154,7 @@ int cg_read_event( if (!streq(key, event)) continue; - val = strdup(q); - if (!val) - return -ENOMEM; - - *ret = TAKE_PTR(val); - return 0; + return strdup_to(ret, q); } } @@ -234,20 +229,13 @@ int cg_read_subgroup(DIR *d, char **ret) { assert(ret); FOREACH_DIRENT_ALL(de, d, return -errno) { - char *b; - if (de->d_type != DT_DIR) continue; if (dot_or_dot_dot(de->d_name)) continue; - b = strdup(de->d_name); - if (!b) - return -ENOMEM; - - *ret = b; - return 1; + return strdup_to_full(ret, de->d_name); } *ret = NULL; @@ -1125,44 +1113,29 @@ int cg_pid_get_path_shifted(pid_t pid, const char *root, char **ret_cgroup) { if (r < 0) return r; - if (c == raw) + if (c == raw) { *ret_cgroup = TAKE_PTR(raw); - else { - char *n; - - n = strdup(c); - if (!n) - return -ENOMEM; - - *ret_cgroup = n; + return 0; } - return 0; + return strdup_to(ret_cgroup, c); } int cg_path_decode_unit(const char *cgroup, char **ret_unit) { - char *c, *s; - size_t n; - assert(cgroup); assert(ret_unit); - n = strcspn(cgroup, "/"); + size_t n = strcspn(cgroup, "/"); if (n < 3) return -ENXIO; - c = strndupa_safe(cgroup, n); + char *c = strndupa_safe(cgroup, n); c = cg_unescape(c); if (!unit_name_is_valid(c, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE)) return -ENXIO; - s = strdup(c); - if (!s) - return -ENOMEM; - - *ret_unit = s; - return 0; + return strdup_to(ret_unit, c); } static bool valid_slice_name(const char *p, size_t n) { @@ -1467,17 +1440,10 @@ int cg_path_get_session(const char *path, char **ret_session) { if (!session_id_valid(start)) return -ENXIO; - if (ret_session) { - char *rr; + if (!ret_session) + return 0; - rr = strdup(start); - if (!rr) - return -ENOMEM; - - *ret_session = rr; - } - - return 0; + return strdup_to(ret_session, start); } int cg_pid_get_session(pid_t pid, char **ret_session) { @@ -1538,30 +1504,19 @@ int cg_path_get_slice(const char *p, char **ret_slice) { * stops before we come to the first non-slice unit. */ for (;;) { - size_t n; - p += strspn(p, "/"); - n = strcspn(p, "/"); - if (!valid_slice_name(p, n)) { - - if (!e) { - char *s; - - s = strdup(SPECIAL_ROOT_SLICE); - if (!s) - return -ENOMEM; - - *ret_slice = s; - return 0; - } - - return cg_path_decode_unit(e, ret_slice); - } + size_t n = strcspn(p, "/"); + if (!valid_slice_name(p, n)) + break; e = p; p += n; } + + if (e) + return cg_path_decode_unit(e, ret_slice); + return strdup_to(ret_slice, SPECIAL_ROOT_SLICE); } int cg_pid_get_slice(pid_t pid, char **ret_slice) { @@ -1714,15 +1669,8 @@ int cg_slice_to_path(const char *unit, char **ret) { assert(unit); assert(ret); - if (streq(unit, SPECIAL_ROOT_SLICE)) { - char *x; - - x = strdup(""); - if (!x) - return -ENOMEM; - *ret = x; - return 0; - } + if (streq(unit, SPECIAL_ROOT_SLICE)) + return strdup_to(ret, ""); if (!unit_name_is_valid(unit, UNIT_NAME_PLAIN)) return -EINVAL; From 7e850da3d68821ff75d17a81dfb97fedc59b1a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 Mar 2024 13:41:05 +0100 Subject: [PATCH 05/25] test-cgroup-util: reduce scope of iterator variables --- src/test/test-cgroup-util.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index cb5a981dc3..4e7805b40a 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -328,12 +328,13 @@ TEST(shift_path) { TEST(mask_supported, .sd_booted = true) { CGroupMask m; - CGroupController c; assert_se(cg_mask_supported(&m) >= 0); - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) - printf("'%s' is supported: %s\n", cgroup_controller_to_string(c), yes_no(m & CGROUP_CONTROLLER_TO_MASK(c))); + for (CGroupController c = 0; c < _CGROUP_CONTROLLER_MAX; c++) + printf("'%s' is supported: %s\n", + cgroup_controller_to_string(c), + yes_no(m & CGROUP_CONTROLLER_TO_MASK(c))); } TEST(is_cgroup_fs, .sd_booted = true) { @@ -392,7 +393,7 @@ TEST(cg_tests) { TEST(cg_get_keyed_attribute) { _cleanup_free_ char *val = NULL; char *vals3[3] = {}, *vals3a[3] = {}; - int i, r; + int r; r = cg_get_keyed_attribute("cpu", "/init.scope", "no_such_file", STRV_MAKE("no_such_attr"), &val); if (IN_SET(r, -ENOMEDIUM, -ENOENT) || ERRNO_IS_PRIVILEGE(r)) { @@ -430,7 +431,7 @@ TEST(cg_get_keyed_attribute) { assert_se(cg_get_keyed_attribute("cpu", "/init.scope", "cpu.stat", STRV_MAKE("usage_usec", "user_usec", "system_usec"), vals3) == 0); - for (i = 0; i < 3; i++) + for (size_t i = 0; i < 3; i++) free(vals3[i]); assert_se(cg_get_keyed_attribute_graceful("cpu", "/init.scope", "cpu.stat", @@ -440,7 +441,7 @@ TEST(cg_get_keyed_attribute) { assert_se(cg_get_keyed_attribute("cpu", "/init.scope", "cpu.stat", STRV_MAKE("system_usec", "user_usec", "usage_usec"), vals3a) == 0); - for (i = 0; i < 3; i++) + for (size_t i = 0; i < 3; i++) free(vals3a[i]); assert_se(cg_get_keyed_attribute_graceful("cpu", "/init.scope", "cpu.stat", @@ -448,7 +449,7 @@ TEST(cg_get_keyed_attribute) { log_info("cpu /init.scope cpu.stat [system_usec user_usec usage_usec] → \"%s\", \"%s\", \"%s\"", vals3a[0], vals3a[1], vals3a[2]); - for (i = 0; i < 3; i++) { + for (size_t i = 0; i < 3; i++) { free(vals3[i]); free(vals3a[i]); } From 454318d3e86c125a78e61a825d84db1b52c1a4a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 18:27:04 +0100 Subject: [PATCH 06/25] various: use strdup_to() in various obvious cases strdup_to() returns 0 on success and here we convert obvious blocks which either return -ENOMEM or 0. --- src/basic/chase.c | 7 +------ src/basic/devnum-util.c | 11 ++++------- src/basic/format-util.c | 9 ++------- src/basic/path-util.c | 9 ++------- src/basic/terminal-util.c | 23 +++++------------------ src/basic/unit-name.c | 7 +------ src/home/homed-home.c | 8 +------- src/locale/localed.c | 6 +----- src/login/loginctl.c | 8 +------- src/login/logind-session-device.c | 6 +----- src/resolve/resolved-dnssd.c | 8 +------- src/shared/ethtool-util.c | 8 +------- src/shared/install.c | 12 +++--------- src/shared/install.h | 2 +- src/shared/libcrypt-util.c | 9 ++------- src/shared/netif-util.c | 10 ++-------- src/shared/pretty-print.c | 12 ++---------- 17 files changed, 31 insertions(+), 124 deletions(-) diff --git a/src/basic/chase.c b/src/basic/chase.c index 9f5477e4f3..245dd0800e 100644 --- a/src/basic/chase.c +++ b/src/basic/chase.c @@ -741,12 +741,7 @@ int chase_extract_filename(const char *path, const char *root, char **ret) { return r; } - char *fname = strdup("."); - if (!fname) - return -ENOMEM; - - *ret = fname; - return 0; + return strdup_to(ret, "."); } int chase_and_open(const char *path, const char *root, ChaseFlags chase_flags, int open_flags, char **ret_path) { diff --git a/src/basic/devnum-util.c b/src/basic/devnum-util.c index f82e13bdb0..652740cea7 100644 --- a/src/basic/devnum-util.c +++ b/src/basic/devnum-util.c @@ -58,21 +58,18 @@ int device_path_make_major_minor(mode_t mode, dev_t devnum, char **ret) { } int device_path_make_inaccessible(mode_t mode, char **ret) { - char *s; + const char *s; assert(ret); if (S_ISCHR(mode)) - s = strdup("/run/systemd/inaccessible/chr"); + s = "/run/systemd/inaccessible/chr"; else if (S_ISBLK(mode)) - s = strdup("/run/systemd/inaccessible/blk"); + s = "/run/systemd/inaccessible/blk"; else return -ENODEV; - if (!s) - return -ENOMEM; - *ret = s; - return 0; + return strdup_to(ret, s); } int device_path_make_canonical(mode_t mode, dev_t devnum, char **ret) { diff --git a/src/basic/format-util.c b/src/basic/format-util.c index 94501853a1..056c990cc7 100644 --- a/src/basic/format-util.c +++ b/src/basic/format-util.c @@ -25,7 +25,7 @@ int format_ifname_full(int ifindex, FormatIfnameFlag flag, char buf[static IF_NA } int format_ifname_full_alloc(int ifindex, FormatIfnameFlag flag, char **ret) { - char buf[IF_NAMESIZE], *copy; + char buf[IF_NAMESIZE]; int r; assert(ret); @@ -34,12 +34,7 @@ int format_ifname_full_alloc(int ifindex, FormatIfnameFlag flag, char **ret) { if (r < 0) return r; - copy = strdup(buf); - if (!copy) - return -ENOMEM; - - *ret = copy; - return 0; + return strdup_to(ret, buf); } char *format_bytes_full(char *buf, size_t l, uint64_t t, FormatBytesFlag flag) { diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 05a21f8f8e..4d335c64ee 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -1094,7 +1094,6 @@ int path_extract_filename(const char *path, char **ret) { } int path_extract_directory(const char *path, char **ret) { - _cleanup_free_ char *a = NULL; const char *c, *next = NULL; int r; @@ -1118,14 +1117,10 @@ int path_extract_directory(const char *path, char **ret) { if (*path != '/') /* filename only */ return -EDESTADDRREQ; - a = strdup("/"); - if (!a) - return -ENOMEM; - *ret = TAKE_PTR(a); - return 0; + return strdup_to(ret, "/"); } - a = strndup(path, next - path); + _cleanup_free_ char *a = strndup(path, next - path); if (!a) return -ENOMEM; diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 8b5a9fa8c6..fd43f92491 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -697,18 +697,10 @@ int vtnr_from_tty(const char *tty) { tty = active; } - if (tty == active) - *ret = TAKE_PTR(active); - else { - char *tmp; - - tmp = strdup(tty); - if (!tmp) - return -ENOMEM; - - *ret = tmp; - } + if (tty != active) + return strdup_to(ret, tty); + *ret = TAKE_PTR(active); return 0; } @@ -988,7 +980,7 @@ bool on_tty(void) { } int getttyname_malloc(int fd, char **ret) { - char path[PATH_MAX], *c; /* PATH_MAX is counted *with* the trailing NUL byte */ + char path[PATH_MAX]; /* PATH_MAX is counted *with* the trailing NUL byte */ int r; assert(fd >= 0); @@ -1001,12 +993,7 @@ int getttyname_malloc(int fd, char **ret) { if (r > 0) return -r; - c = strdup(skip_dev_prefix(path)); - if (!c) - return -ENOMEM; - - *ret = c; - return 0; + return strdup_to(ret, skip_dev_prefix(path)); } int getttyname_harder(int fd, char **ret) { diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 06ed9054d0..a84b14568d 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -793,12 +793,7 @@ int unit_name_mangle_with_suffix( return 1; good: - s = strdup(name); - if (!s) - return -ENOMEM; - - *ret = TAKE_PTR(s); - return 0; + return strdup_to(ret, name); } int slice_build_parent_slice(const char *slice, char **ret) { diff --git a/src/home/homed-home.c b/src/home/homed-home.c index e0c8637c4c..e1379f51f5 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -3145,7 +3145,6 @@ int home_schedule_operation(Home *h, Operation *o, sd_bus_error *error) { static int home_get_image_path_seat(Home *h, char **ret) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; - _cleanup_free_ char *c = NULL; const char *ip, *seat; struct stat st; int r; @@ -3178,12 +3177,7 @@ static int home_get_image_path_seat(Home *h, char **ret) { else if (r < 0) return r; - c = strdup(seat); - if (!c) - return -ENOMEM; - - *ret = TAKE_PTR(c); - return 0; + return strdup_to(ret, seat); } int home_auto_login(Home *h, char ***ret_seats) { diff --git a/src/locale/localed.c b/src/locale/localed.c index 5758a2624c..c0d104578d 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -158,11 +158,7 @@ static int process_locale_list_item( if (new_locale[p]) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Locale variable %s set twice, refusing.", name); - new_locale[p] = strdup(e); - if (!new_locale[p]) - return -ENOMEM; - - return 0; + return strdup_to(&new_locale[p], e); } return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Locale assignment %s not valid, refusing.", assignment); diff --git a/src/login/loginctl.c b/src/login/loginctl.c index 63253bd9dc..8236dc2dc8 100644 --- a/src/login/loginctl.c +++ b/src/login/loginctl.c @@ -1028,7 +1028,6 @@ static int get_bus_path_by_id( _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - _cleanup_free_ char *p = NULL; const char *path; int r; @@ -1047,12 +1046,7 @@ static int get_bus_path_by_id( if (r < 0) return bus_log_parse_error(r); - p = strdup(path); - if (!p) - return log_oom(); - - *ret = TAKE_PTR(p); - return 0; + return strdup_to(ret, path); } static int show_session(int argc, char *argv[], void *userdata) { diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index cd5c0ec82d..c924f1e0ca 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -314,11 +314,7 @@ static int session_device_verify(SessionDevice *sd) { if (sd->device->seat != sd->session->seat) return -EPERM; - sd->node = strdup(node); - if (!sd->node) - return -ENOMEM; - - return 0; + return strdup_to(&sd->node, node); } int session_device_new(Session *s, dev_t dev, bool open_device, SessionDevice **ret) { diff --git a/src/resolve/resolved-dnssd.c b/src/resolve/resolved-dnssd.c index 404c290eec..8790755d3b 100644 --- a/src/resolve/resolved-dnssd.c +++ b/src/resolve/resolved-dnssd.c @@ -140,16 +140,10 @@ static int dnssd_service_load(Manager *manager, const char *filename) { static int specifier_dnssd_hostname(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const Manager *m = ASSERT_PTR(userdata); - char *n; assert(m->llmnr_hostname); - n = strdup(m->llmnr_hostname); - if (!n) - return -ENOMEM; - - *ret = n; - return 0; + return strdup_to(ret, m->llmnr_hostname); } int dnssd_render_instance_name(Manager *m, DnssdService *s, char **ret) { diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index dce9e005fe..1e100c35ef 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -182,7 +182,6 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { struct ifreq ifr = { .ifr_data = (void*) &ecmd, }; - char *d; int r; assert(ethtool_fd); @@ -201,12 +200,7 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { if (isempty(ecmd.driver)) return -ENODATA; - d = strdup(ecmd.driver); - if (!d) - return -ENOMEM; - - *ret = d; - return 0; + return strdup_to(ret, ecmd.driver); } int ethtool_get_link_info( diff --git a/src/shared/install.c b/src/shared/install.c index 12bf083a2e..47dba1f11c 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3042,17 +3042,16 @@ int unit_file_set_default( int unit_file_get_default( RuntimeScope scope, const char *root_dir, - char **name) { + char **ret) { _cleanup_(lookup_paths_done) LookupPaths lp = {}; _cleanup_(install_context_done) InstallContext ctx = { .scope = scope }; InstallInfo *info; - char *n; int r; assert(scope >= 0); assert(scope < _RUNTIME_SCOPE_MAX); - assert(name); + assert(ret); r = lookup_paths_init(&lp, scope, 0, root_dir); if (r < 0) @@ -3063,12 +3062,7 @@ int unit_file_get_default( if (r < 0) return r; - n = strdup(info->name); - if (!n) - return -ENOMEM; - - *name = n; - return 0; + return strdup_to(ret, info->name); } int unit_file_lookup_state( diff --git a/src/shared/install.h b/src/shared/install.h index a09557f69f..8cce6dddfe 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -175,7 +175,7 @@ int unit_file_set_default( int unit_file_get_default( RuntimeScope scope, const char *root_dir, - char **name); + char **ret); int unit_file_add_dependency( RuntimeScope scope, UnitFileFlags flags, diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c index 81e6f1754c..2f3abe4e89 100644 --- a/src/shared/libcrypt-util.c +++ b/src/shared/libcrypt-util.c @@ -140,7 +140,7 @@ static char* systemd_crypt_ra(const char *phrase, const char *setting, void **da int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret) { _cleanup_free_ char *salt = NULL; _cleanup_(erase_and_freep) void *_cd_data = NULL; - char *p; + const char *p; int r, _cd_size = 0; assert(!!cd_data == !!cd_size); @@ -155,12 +155,7 @@ int hash_password_full(const char *password, void **cd_data, int *cd_size, char return log_debug_errno(errno_or_else(SYNTHETIC_ERRNO(EINVAL)), CRYPT_RA_NAME "() failed: %m"); - p = strdup(p); - if (!p) - return -ENOMEM; - - *ret = p; - return 0; + return strdup_to(ret, p); } bool looks_like_hashed_password(const char *s) { diff --git a/src/shared/netif-util.c b/src/shared/netif-util.c index f56c5646c1..c9a14b4739 100644 --- a/src/shared/netif-util.c +++ b/src/shared/netif-util.c @@ -32,14 +32,8 @@ int net_get_type_string(sd_device *device, uint16_t iftype, char **ret) { if (device && sd_device_get_devtype(device, &t) >= 0 && - !isempty(t)) { - p = strdup(t); - if (!p) - return -ENOMEM; - - *ret = p; - return 0; - } + !isempty(t)) + return strdup_to(ret, t); t = arphrd_to_name(iftype); if (!t) diff --git a/src/shared/pretty-print.c b/src/shared/pretty-print.c index c551ed62e3..85c599c5c6 100644 --- a/src/shared/pretty-print.c +++ b/src/shared/pretty-print.c @@ -141,16 +141,8 @@ int terminal_urlify_path(const char *path, const char *text, char **ret) { if (isempty(text)) text = path; - if (!urlify_enabled()) { - char *n; - - n = strdup(text); - if (!n) - return -ENOMEM; - - *ret = n; - return 0; - } + if (!urlify_enabled()) + return strdup_to(ret, text); r = file_url_from_path(path, &url); if (r < 0) From 418b936d477e29b9a95316ad3d9b247ef462ec89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 20:34:42 +0100 Subject: [PATCH 07/25] various: use strdup_to() after getenv() --- src/basic/efivars.c | 12 ++---------- src/basic/path-lookup.c | 17 +++++------------ src/basic/proc-cmdline.c | 12 ++---------- src/basic/process-util.c | 29 ++++------------------------- 4 files changed, 13 insertions(+), 57 deletions(-) diff --git a/src/basic/efivars.c b/src/basic/efivars.c index 9011ae29a3..fdc6c439bb 100644 --- a/src/basic/efivars.c +++ b/src/basic/efivars.c @@ -398,16 +398,8 @@ int systemd_efi_options_variable(char **ret) { /* For testing purposes it is sometimes useful to be able to override this */ e = secure_getenv("SYSTEMD_EFI_OPTIONS"); - if (e) { - char *m; - - m = strdup(e); - if (!m) - return -ENOMEM; - - *ret = m; - return 0; - } + if (e) + return strdup_to(ret, e); r = read_one_line_file(EFIVAR_CACHE_PATH(EFI_SYSTEMD_VARIABLE(SystemdOptions)), ret); if (r == -ENOENT) diff --git a/src/basic/path-lookup.c b/src/basic/path-lookup.c index e7fc4a7f06..540256b73b 100644 --- a/src/basic/path-lookup.c +++ b/src/basic/path-lookup.c @@ -92,7 +92,6 @@ int xdg_user_data_dir(char **ret, const char *suffix) { } int runtime_directory(char **ret, RuntimeScope scope, const char *suffix) { - _cleanup_free_ char *d = NULL; int r; assert(ret); @@ -106,26 +105,20 @@ int runtime_directory(char **ret, RuntimeScope scope, const char *suffix) { * Return value indicates whether the suffix was applied or not */ const char *e = secure_getenv("RUNTIME_DIRECTORY"); - if (e) { - d = strdup(e); - if (!d) - return -ENOMEM; - - *ret = TAKE_PTR(d); - return false; - } + if (e) + return strdup_to(ret, e); if (scope == RUNTIME_SCOPE_USER) { - r = xdg_user_runtime_dir(&d, suffix); + r = xdg_user_runtime_dir(ret, suffix); if (r < 0) return r; } else { - d = path_join("/run", suffix); + char *d = path_join("/run", suffix); if (!d) return -ENOMEM; + *ret = d; } - *ret = TAKE_PTR(d); return true; } diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 522d8de1f4..ce1ba3a1ea 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -116,16 +116,8 @@ int proc_cmdline(char **ret) { /* For testing purposes it is sometimes useful to be able to override what we consider /proc/cmdline to be */ e = secure_getenv("SYSTEMD_PROC_CMDLINE"); - if (e) { - char *m; - - m = strdup(e); - if (!m) - return -ENOMEM; - - *ret = m; - return 0; - } + if (e) + return strdup_to(ret, e); if (detect_container() > 0) return pid_get_cmdline(1, SIZE_MAX, 0, ret); diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 69635e65f8..351b5e4095 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1022,7 +1022,6 @@ int kill_and_sigcont(pid_t pid, int sig) { int getenv_for_pid(pid_t pid, const char *field, char **ret) { _cleanup_fclose_ FILE *f = NULL; - char *value = NULL; const char *path; size_t sum = 0; int r; @@ -1031,22 +1030,8 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) { assert(field); assert(ret); - if (pid == 0 || pid == getpid_cached()) { - const char *e; - - e = getenv(field); - if (!e) { - *ret = NULL; - return 0; - } - - value = strdup(e); - if (!value) - return -ENOMEM; - - *ret = value; - return 1; - } + if (pid == 0 || pid == getpid_cached()) + return strdup_to_full(ret, getenv(field)); if (!pid_is_valid(pid)) return -EINVAL; @@ -1075,14 +1060,8 @@ int getenv_for_pid(pid_t pid, const char *field, char **ret) { sum += r; match = startswith(line, field); - if (match && *match == '=') { - value = strdup(match + 1); - if (!value) - return -ENOMEM; - - *ret = value; - return 1; - } + if (match && *match == '=') + return strdup_to_full(ret, match + 1); } *ret = NULL; From 4f77ddca591a25779407a89ca959d8b0176b2d4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:23:26 +0100 Subject: [PATCH 08/25] various: also use strdup_to() in cases where we don't return immediately --- src/basic/terminal-util.c | 10 +++------- src/home/homework-luks.c | 10 +++------- src/shared/install.c | 8 +++----- src/shared/password-quality-util-pwquality.c | 11 ++++------- 4 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index fd43f92491..518412ec36 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1098,13 +1098,9 @@ int get_ctty(pid_t pid, dev_t *ret_devnr, char **ret) { return -EINVAL; if (ret) { - _cleanup_free_ char *b = NULL; - - b = strdup(w); - if (!b) - return -ENOMEM; - - *ret = TAKE_PTR(b); + r = strdup_to(ret, w); + if (r < 0) + return r; } if (ret_devnr) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 9d86881d15..8dcb5c5d46 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -126,7 +126,6 @@ static int probe_file_system_by_fd( sd_id128_t *ret_uuid) { _cleanup_(blkid_free_probep) blkid_probe b = NULL; - _cleanup_free_ char *s = NULL; const char *fstype = NULL, *uuid = NULL; sd_id128_t id; int r; @@ -168,13 +167,10 @@ static int probe_file_system_by_fd( if (r < 0) return r; - s = strdup(fstype); - if (!s) - return -ENOMEM; - - *ret_fstype = TAKE_PTR(s); + r = strdup_to(ret_fstype, fstype); + if (r < 0) + return r; *ret_uuid = id; - return 0; } diff --git a/src/shared/install.c b/src/shared/install.c index 47dba1f11c..8fe0165225 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -3215,11 +3215,9 @@ int unit_file_exists_full(RuntimeScope scope, const LookupPaths *lp, const char if (ret_path) { assert(info); - _cleanup_free_ char *p = strdup(info->path); - if (!p) - return -ENOMEM; - - *ret_path = TAKE_PTR(p); + r = strdup_to(ret_path, info->path); + if (r < 0) + return r; } return 1; diff --git a/src/shared/password-quality-util-pwquality.c b/src/shared/password-quality-util-pwquality.c index 87e7d285c5..60816f8245 100644 --- a/src/shared/password-quality-util-pwquality.c +++ b/src/shared/password-quality-util-pwquality.c @@ -145,13 +145,10 @@ int check_password_quality(const char *password, const char *old, const char *us r = sym_pwquality_check(pwq, password, old, username, &auxerror); if (r < 0) { if (ret_error) { - _cleanup_free_ char *e = NULL; - - e = strdup(sym_pwquality_strerror(buf, sizeof(buf), r, auxerror)); - if (!e) - return -ENOMEM; - - *ret_error = TAKE_PTR(e); + r = strdup_to(ret_error, + sym_pwquality_strerror(buf, sizeof(buf), r, auxerror)); + if (r < 0) + return r; } return 0; /* all bad */ From 94b75cdb06aae1e5fe91f5846dbed5781f8d5ecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 18:29:40 +0100 Subject: [PATCH 09/25] basic/fileio: use strdup_to_full() in read_stripped_line() The return value of read_stripped_line() is changed. Before we'd return the number of characters read, but that number was not meaningful after we called strstrip(). So just return 0 if nothing was read (EOF), and 1 if something was read (not EOF). All the callers were only checking for <0 or ==0. --- src/basic/fileio.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 3aadb4bfab..523378177f 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -1504,7 +1504,7 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) { int read_stripped_line(FILE *f, size_t limit, char **ret) { _cleanup_free_ char *s = NULL; - int r; + int r, k; assert(f); @@ -1513,23 +1513,17 @@ int read_stripped_line(FILE *f, size_t limit, char **ret) { return r; if (ret) { - const char *p; - - p = strstrip(s); + const char *p = strstrip(s); if (p == s) *ret = TAKE_PTR(s); else { - char *copy; - - copy = strdup(p); - if (!copy) - return -ENOMEM; - - *ret = copy; + k = strdup_to(ret, p); + if (k < 0) + return k; } } - return r; + return r > 0; /* Return 1 if something was read. */ } int safe_fgetc(FILE *f, char *ret) { From 55222b75a626be039b71484ae1fbee950deab6b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 18:41:08 +0100 Subject: [PATCH 10/25] systemd-analyze: use strdup_to() --- src/analyze/analyze-security.c | 98 +++++++++++++++------------------- 1 file changed, 44 insertions(+), 54 deletions(-) diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index 030b206f1e..c1b35ba852 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -215,20 +215,21 @@ static int assess_user( uint64_t *ret_badness, char **ret_description) { - _cleanup_free_ char *d = NULL; + const char *d; uint64_t b; + int r; assert(ret_badness); assert(ret_description); if (streq_ptr(info->user, NOBODY_USER_NAME)) { - d = strdup("Service runs under as '" NOBODY_USER_NAME "' user, which should not be used for services"); + d = "Service runs under as '" NOBODY_USER_NAME "' user, which should not be used for services"; b = 9; } else if (info->dynamic_user && !STR_IN_SET(info->user, "0", "root")) { - d = strdup("Service runs under a transient non-root user identity"); + d = "Service runs under a transient non-root user identity"; b = 0; } else if (info->user && !STR_IN_SET(info->user, "0", "root", "")) { - d = strdup("Service runs under a static non-root user identity"); + d = "Service runs under a static non-root user identity"; b = 0; } else { *ret_badness = 10; @@ -236,12 +237,11 @@ static int assess_user( return 0; } - if (!d) - return log_oom(); + r = strdup_to(ret_description, d); + if (r < 0) + return r; *ret_badness = b; - *ret_description = TAKE_PTR(d); - return 0; } @@ -254,7 +254,6 @@ static int assess_protect_home( const char *description; uint64_t badness; - char *copy; int r; assert(ret_badness); @@ -277,13 +276,11 @@ static int assess_protect_home( description = "Service has no access to home directories"; } - copy = strdup(description); - if (!copy) - return log_oom(); + r = strdup_to(ret_description, description); + if (r < 0) + return r; *ret_badness = badness; - *ret_description = copy; - return 0; } @@ -296,7 +293,6 @@ static int assess_protect_system( const char *description; uint64_t badness; - char *copy; int r; assert(ret_badness); @@ -319,13 +315,11 @@ static int assess_protect_system( description = "Service has limited write access to the OS file hierarchy"; } - copy = strdup(description); - if (!copy) - return log_oom(); + r = strdup_to(ret_description, description); + if (r < 0) + return r; *ret_badness = badness; - *ret_description = copy; - return 0; } @@ -370,9 +364,9 @@ static int assess_umask( uint64_t *ret_badness, char **ret_description) { - char *copy = NULL; const char *d; uint64_t b; + int r; assert(ret_badness); assert(ret_description); @@ -394,13 +388,11 @@ static int assess_umask( b = 0; } - copy = strdup(d); - if (!copy) - return log_oom(); + r = strdup_to(ret_description, d); + if (r < 0) + return r; *ret_badness = b; - *ret_description = copy; - return 0; } @@ -537,30 +529,30 @@ static int assess_system_call_architectures( uint64_t *ret_badness, char **ret_description) { - char *d; + const char *d; uint64_t b; + int r; assert(ret_badness); assert(ret_description); if (set_isempty(info->system_call_architectures)) { b = 10; - d = strdup("Service may execute system calls with all ABIs"); + d = "Service may execute system calls with all ABIs"; } else if (set_contains(info->system_call_architectures, "native") && set_size(info->system_call_architectures) == 1) { b = 0; - d = strdup("Service may execute system calls only with native ABI"); + d = "Service may execute system calls only with native ABI"; } else { b = 8; - d = strdup("Service may execute system calls with multiple ABIs"); + d = "Service may execute system calls with multiple ABIs"; } - if (!d) - return log_oom(); + r = strdup_to(ret_description, d); + if (r < 0) + return r; *ret_badness = b; - *ret_description = d; - return 0; } @@ -607,12 +599,12 @@ static int assess_system_call_filter( assert(a->parameter < _SYSCALL_FILTER_SET_MAX); const SyscallFilterSet *f = syscall_filter_sets + a->parameter; - _cleanup_free_ char *d = NULL; + char *d; uint64_t b; int r; if (!info->system_call_filter_allow_list && set_isempty(info->system_call_filter)) { - r = free_and_strdup(&d, "Service does not filter system calls"); + r = strdup_to(&d, "Service does not filter system calls"); b = 10; } else { bool bad; @@ -649,8 +641,8 @@ static int assess_system_call_filter( if (r < 0) return log_oom(); + *ret_description = d; *ret_badness = b; - *ret_description = TAKE_PTR(d); return 0; } @@ -664,36 +656,36 @@ static int assess_ip_address_allow( uint64_t *ret_badness, char **ret_description) { - char *d = NULL; + const char *d; uint64_t b; + int r; assert(info); assert(ret_badness); assert(ret_description); if (info->ip_filters_custom_ingress || info->ip_filters_custom_egress) { - d = strdup("Service defines custom ingress/egress IP filters with BPF programs"); + d = "Service defines custom ingress/egress IP filters with BPF programs"; b = 0; } else if (!info->ip_address_deny_all) { - d = strdup("Service does not define an IP address allow list"); + d = "Service does not define an IP address allow list"; b = 10; } else if (info->ip_address_allow_other) { - d = strdup("Service defines IP address allow list with non-localhost entries"); + d = "Service defines IP address allow list with non-localhost entries"; b = 5; } else if (info->ip_address_allow_localhost) { - d = strdup("Service defines IP address allow list with only localhost entries"); + d = "Service defines IP address allow list with only localhost entries"; b = 2; } else { - d = strdup("Service blocks all IP address ranges"); + d = "Service blocks all IP address ranges"; b = 0; } - if (!d) - return log_oom(); + r = strdup_to(ret_description, d); + if (r < 0) + return r; *ret_badness = b; - *ret_description = d; - return 0; } @@ -704,7 +696,7 @@ static int assess_device_allow( uint64_t *ret_badness, char **ret_description) { - char *d = NULL; + char *d; uint64_t b; assert(info); @@ -2828,7 +2820,6 @@ static int analyze_security(sd_bus *bus, for (;;) { UnitInfo info; - char *copy = NULL; r = bus_parse_unit_info(reply, &info); if (r < 0) @@ -2842,12 +2833,11 @@ static int analyze_security(sd_bus *bus, if (!GREEDY_REALLOC(list, n + 2)) return log_oom(); - copy = strdup(info.id); - if (!copy) - return log_oom(); + r = strdup_to(&list[n], info.id); + if (r < 0) + return r; - list[n++] = copy; - list[n] = NULL; + list[++n] = NULL; } strv_sort(list); From f394876140c06ed174e89bcac08b7298dfdeaff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:50:08 +0100 Subject: [PATCH 11/25] coredump: use free_and_strdup_warn() more --- src/coredump/coredump-vacuum.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/coredump/coredump-vacuum.c b/src/coredump/coredump-vacuum.c index 7e0c98cb7d..8e2febd581 100644 --- a/src/coredump/coredump-vacuum.c +++ b/src/coredump/coredump-vacuum.c @@ -179,18 +179,12 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { c = hashmap_get(h, UID_TO_PTR(uid)); if (c) { - if (t < c->oldest_mtime) { - char *n; - - n = strdup(de->d_name); - if (!n) - return log_oom(); - - free_and_replace(c->oldest_file, n); + r = free_and_strdup_warn(&c->oldest_file, de->d_name); + if (r < 0) + return r; c->oldest_mtime = t; } - } else { _cleanup_(vacuum_candidate_freep) VacuumCandidate *n = NULL; @@ -198,10 +192,9 @@ int coredump_vacuum(int exclude_fd, uint64_t keep_free, uint64_t max_use) { if (!n) return log_oom(); - n->oldest_file = strdup(de->d_name); - if (!n->oldest_file) - return log_oom(); - + r = free_and_strdup_warn(&n->oldest_file, de->d_name); + if (r < 0) + return r; n->oldest_mtime = t; r = hashmap_put(h, UID_TO_PTR(uid), n); From 4eb3ec63b06d8dbeef77875db2be12e161515992 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 20:40:57 +0100 Subject: [PATCH 12/25] basic/socket-util: use strdup_to() in sockaddr_pretty() The function was written to support ret==NULL, but the only caller always passes ret, and sockaddr_pretty() also requires ret to be set, so that half-implemented code wasn't very useful. --- src/basic/socket-util.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 68e6afc67f..5eb8e4c988 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -453,6 +453,7 @@ int sockaddr_pretty( assert(sa); assert(salen >= sizeof(sa->sa.sa_family)); + assert(ret); switch (sa->sa.sa_family) { @@ -634,6 +635,7 @@ int socknameinfo_pretty(const struct sockaddr *sa, socklen_t salen, char **ret) assert(sa); assert(salen > sizeof(sa_family_t)); + assert(ret); r = getnameinfo(sa, salen, host, sizeof(host), /* service= */ NULL, /* service_len= */ 0, IDN_FLAGS); if (r != 0) { @@ -647,15 +649,7 @@ int socknameinfo_pretty(const struct sockaddr *sa, socklen_t salen, char **ret) return sockaddr_pretty(sa, salen, /* translate_ipv6= */ true, /* include_port= */ true, ret); } - if (ret) { - char *copy = strdup(host); - if (!copy) - return -ENOMEM; - - *ret = copy; - } - - return 0; + return strdup_to(ret, host); } static const char* const netlink_family_table[] = { From f174b294f64281b57bf1634cf2140fe3ca22d1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 20:47:12 +0100 Subject: [PATCH 13/25] basic/string-util: use strdup_to() in string_extract_line() --- src/basic/string-util.c | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 6e8d10747b..7f6c87b461 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1253,36 +1253,15 @@ int string_extract_line(const char *s, size_t i, char **ret) { return -ENOMEM; *ret = m; - return !isempty(q + 1); /* more coming? */ - } else { - if (p == s) - *ret = NULL; /* Just use the input string */ - else { - char *m; - - m = strdup(p); - if (!m) - return -ENOMEM; - - *ret = m; - } - - return 0; /* The end */ - } + return !isempty(q + 1); /* More coming? */ + } else + /* Tell the caller to use the input string if equal */ + return strdup_to(ret, p != s ? p : NULL); } - if (!q) { - char *m; - + if (!q) /* No more lines, return empty line */ - - m = strdup(""); - if (!m) - return -ENOMEM; - - *ret = m; - return 0; /* The end */ - } + return strdup_to(ret, ""); p = q + 1; c++; From 6a705f1234919c255cc230403ad6749edabf83be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 21:08:52 +0100 Subject: [PATCH 14/25] Replace strdup_or_null() by strdup_to() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I didn't know that this helper existed… It is very similar to strdup_to_full(), but all callers can actually be replaced by strdup_to(), which has more fitting semantics. --- src/basic/string-util.h | 21 --------------------- src/core/unit.c | 2 +- src/fstab-generator/fstab-generator.c | 9 ++------- src/kernel-install/kernel-install.c | 20 ++++++++++---------- src/network/networkd-address.c | 4 ++-- src/network/networkd-route-metric.c | 2 +- src/network/networkd-route-nexthop.c | 2 +- 7 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/basic/string-util.h b/src/basic/string-util.h index c402cd3af0..a68f3c94ab 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -303,25 +303,4 @@ bool version_is_valid_versionspec(const char *s); ssize_t strlevenshtein(const char *x, const char *y); -static inline int strdup_or_null(const char *s, char **ret) { - char *c; - - assert(ret); - - /* This is a lot like strdup(), but is happy with NULL strings, and does not treat that as error, but - * copies the NULL value. */ - - if (!s) { - *ret = NULL; - return 0; - } - - c = strdup(s); - if (!c) - return -ENOMEM; - - *ret = c; - return 1; -} - char *strrstr(const char *haystack, const char *needle); diff --git a/src/core/unit.c b/src/core/unit.c index c0f802a137..9c0794b886 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5338,7 +5338,7 @@ int unit_set_exec_params(Unit *u, ExecParameters *p) { p->runtime_scope = u->manager->runtime_scope; - r = strdup_or_null(manager_get_confirm_spawn(u->manager), &p->confirm_spawn); + r = strdup_to(&p->confirm_spawn, manager_get_confirm_spawn(u->manager)); if (r < 0) return r; diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index e4c8c9674c..b4df9d23c2 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -498,13 +498,8 @@ static int mandatory_mount_drop_unapplicable_options( assert(where); assert(ret_options); - if (!(*flags & (MOUNT_NOAUTO|MOUNT_NOFAIL|MOUNT_AUTOMOUNT))) { - r = strdup_or_null(options, ret_options); - if (r < 0) - return r; - - return 0; - } + if (!(*flags & (MOUNT_NOAUTO|MOUNT_NOFAIL|MOUNT_AUTOMOUNT))) + return strdup_to(ret_options, options); log_debug("Mount '%s' is mandatory, ignoring 'noauto', 'nofail', and 'x-systemd.automount' options.", where); diff --git a/src/kernel-install/kernel-install.c b/src/kernel-install/kernel-install.c index cf1bb5c3af..6581e80656 100644 --- a/src/kernel-install/kernel-install.c +++ b/src/kernel-install/kernel-install.c @@ -151,37 +151,37 @@ static int context_copy(const Context *source, Context *ret) { return copy.rfd; } - r = strdup_or_null(source->layout_other, ©.layout_other); + r = strdup_to(©.layout_other, source->layout_other); if (r < 0) return r; - r = strdup_or_null(source->conf_root, ©.conf_root); + r = strdup_to(©.conf_root, source->conf_root); if (r < 0) return r; - r = strdup_or_null(source->boot_root, ©.boot_root); + r = strdup_to(©.boot_root, source->boot_root); if (r < 0) return r; - r = strdup_or_null(source->entry_token, ©.entry_token); + r = strdup_to(©.entry_token, source->entry_token); if (r < 0) return r; - r = strdup_or_null(source->entry_dir, ©.entry_dir); + r = strdup_to(©.entry_dir, source->entry_dir); if (r < 0) return r; - r = strdup_or_null(source->version, ©.version); + r = strdup_to(©.version, source->version); if (r < 0) return r; - r = strdup_or_null(source->kernel, ©.kernel); + r = strdup_to(©.kernel, source->kernel); if (r < 0) return r; r = strv_copy_unless_empty(source->initrds, ©.initrds); if (r < 0) return r; - r = strdup_or_null(source->initrd_generator, ©.initrd_generator); + r = strdup_to(©.initrd_generator, source->initrd_generator); if (r < 0) return r; - r = strdup_or_null(source->uki_generator, ©.uki_generator); + r = strdup_to(©.uki_generator, source->uki_generator); if (r < 0) return r; - r = strdup_or_null(source->staging_area, ©.staging_area); + r = strdup_to(©.staging_area, source->staging_area); if (r < 0) return r; r = strv_copy_unless_empty(source->plugins, ©.plugins); diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index d9c3dae85c..38cdadcf61 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -613,12 +613,12 @@ int address_dup(const Address *src, Address **ret) { dest->nft_set_context.n_sets = 0; if (src->family == AF_INET) { - r = strdup_or_null(src->label, &dest->label); + r = strdup_to(&dest->label, src->label); if (r < 0) return r; } - r = strdup_or_null(src->netlabel, &dest->netlabel); + r = strdup_to(&dest->netlabel, src->netlabel); if (r < 0) return r; diff --git a/src/network/networkd-route-metric.c b/src/network/networkd-route-metric.c index 1bae893934..31a2bdea24 100644 --- a/src/network/networkd-route-metric.c +++ b/src/network/networkd-route-metric.c @@ -39,7 +39,7 @@ int route_metric_copy(const RouteMetric *src, RouteMetric *dest) { } else dest->metrics_set = NULL; - return strdup_or_null(src->tcp_congestion_control_algo, &dest->tcp_congestion_control_algo); + return strdup_to(&dest->tcp_congestion_control_algo, src->tcp_congestion_control_algo); } void route_metric_hash_func(const RouteMetric *metric, struct siphash *state) { diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 8ba7505987..11215c36c7 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -218,7 +218,7 @@ static int route_nexthop_copy(const RouteNextHop *src, RouteNextHop *dest) { /* unset pointer copied in the above. */ dest->ifname = NULL; - return strdup_or_null(src->ifindex > 0 ? NULL : src->ifname, &dest->ifname); + return strdup_to(&dest->ifname, src->ifindex > 0 ? NULL : src->ifname); } static int route_nexthop_dup(const RouteNextHop *src, RouteNextHop **ret) { From bef2c63a40fb7eb0c308f6a72ef9e972a2763ead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 18:29:40 +0100 Subject: [PATCH 15/25] shared: use strdup_to() in specifier functions --- src/shared/install-printf.c | 17 ++++---------- src/shared/specifier.c | 47 +++++-------------------------------- 2 files changed, 10 insertions(+), 54 deletions(-) diff --git a/src/shared/install-printf.c b/src/shared/install-printf.c index 3cc709308d..bdb44b348f 100644 --- a/src/shared/install-printf.c +++ b/src/shared/install-printf.c @@ -37,16 +37,11 @@ static int specifier_prefix_and_instance(char specifier, const void *data, const static int specifier_name(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const InstallInfo *i = ASSERT_PTR(userdata); - char *ans; if (unit_name_is_valid(i->name, UNIT_NAME_TEMPLATE) && i->default_instance) return unit_name_replace_instance(i->name, i->default_instance, ret); - ans = strdup(i->name); - if (!ans) - return -ENOMEM; - *ret = ans; - return 0; + return strdup_to(ret, i->name); } static int specifier_prefix(char specifier, const void *data, const char *root, const void *userdata, char **ret) { @@ -86,14 +81,10 @@ static int specifier_last_component(char specifier, const void *data, const char return r; dash = strrchr(prefix, '-'); - if (dash) { - dash = strdup(dash + 1); - if (!dash) - return -ENOMEM; - *ret = dash; - } else - *ret = TAKE_PTR(prefix); + if (dash) + return strdup_to(ret, dash + 1); + *ret = TAKE_PTR(prefix); return 0; } diff --git a/src/shared/specifier.c b/src/shared/specifier.c index a56b8365fb..f6739f2c66 100644 --- a/src/shared/specifier.c +++ b/src/shared/specifier.c @@ -111,18 +111,7 @@ int specifier_printf(const char *text, size_t max_length, const Specifier table[ /* Generic handler for simple string replacements */ int specifier_string(char specifier, const void *data, const char *root, const void *userdata, char **ret) { - char *n = NULL; - - assert(ret); - - if (!isempty(data)) { - n = strdup(data); - if (!n) - return -ENOMEM; - } - - *ret = n; - return 0; + return strdup_to(ASSERT_PTR(ret), empty_to_null(data)); } int specifier_real_path(char specifier, const void *data, const char *root, const void *userdata, char **ret) { @@ -249,32 +238,18 @@ int specifier_pretty_hostname(char specifier, const void *data, const char *root int specifier_kernel_release(char specifier, const void *data, const char *root, const void *userdata, char **ret) { struct utsname uts; - char *n; assert(ret); if (uname(&uts) < 0) return -errno; - n = strdup(uts.release); - if (!n) - return -ENOMEM; - - *ret = n; - return 0; + return strdup_to(ret, uts.release); } int specifier_architecture(char specifier, const void *data, const char *root, const void *userdata, char **ret) { - char *t; - - assert(ret); - - t = strdup(architecture_to_string(uname_architecture())); - if (!t) - return -ENOMEM; - - *ret = t; - return 0; + return strdup_to(ASSERT_PTR(ret), + architecture_to_string(uname_architecture())); } /* Note: fields in /etc/os-release might quite possibly be missing, even if everything is entirely valid @@ -420,7 +395,6 @@ int specifier_user_shell(char specifier, const void *data, const char *root, con int specifier_tmp_dir(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const char *p; - char *copy; int r; assert(ret); @@ -432,17 +406,12 @@ int specifier_tmp_dir(char specifier, const void *data, const char *root, const if (r < 0) return r; } - copy = strdup(p); - if (!copy) - return -ENOMEM; - *ret = copy; - return 0; + return strdup_to(ret, p); } int specifier_var_tmp_dir(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const char *p; - char *copy; int r; assert(ret); @@ -454,12 +423,8 @@ int specifier_var_tmp_dir(char specifier, const void *data, const char *root, co if (r < 0) return r; } - copy = strdup(p); - if (!copy) - return -ENOMEM; - *ret = copy; - return 0; + return strdup_to(ret, p); } int specifier_escape_strv(char **l, char ***ret) { From bca4dc29df8617cb8d3d391dc43c1d18fd1c4470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:14:31 +0100 Subject: [PATCH 16/25] sd-device: use strdup_to() and rename output param device_open_from_devnum() returns two things: the devname via an output param, and the fd via the return value. Rename the param to signal that it's not the only return channel. --- src/libsystemd/sd-device/device-util.c | 21 ++++++--------------- src/libsystemd/sd-device/device-util.h | 2 +- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/libsystemd/sd-device/device-util.c b/src/libsystemd/sd-device/device-util.c index 8318373fb2..123629c356 100644 --- a/src/libsystemd/sd-device/device-util.c +++ b/src/libsystemd/sd-device/device-util.c @@ -9,7 +9,6 @@ int devname_from_devnum(mode_t mode, dev_t devnum, char **ret) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; - _cleanup_free_ char *s = NULL; const char *devname; int r; @@ -26,15 +25,10 @@ int devname_from_devnum(mode_t mode, dev_t devnum, char **ret) { if (r < 0) return r; - s = strdup(devname); - if (!s) - return -ENOMEM; - - *ret = TAKE_PTR(s); - return 0; + return strdup_to(ret, devname); } -int device_open_from_devnum(mode_t mode, dev_t devnum, int flags, char **ret) { +int device_open_from_devnum(mode_t mode, dev_t devnum, int flags, char **ret_devname) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; _cleanup_close_ int fd = -EBADF; int r; @@ -47,19 +41,16 @@ int device_open_from_devnum(mode_t mode, dev_t devnum, int flags, char **ret) { if (fd < 0) return fd; - if (ret) { + if (ret_devname) { const char *devname; - char *s; r = sd_device_get_devname(dev, &devname); if (r < 0) return r; - s = strdup(devname); - if (!s) - return -ENOMEM; - - *ret = s; + r = strdup_to(ret_devname, devname); + if (r < 0) + return r; } return TAKE_FD(fd); diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 534a296715..b17993d554 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -100,7 +100,7 @@ static inline int devname_from_stat_rdev(const struct stat *st, char **ret) { assert(st); return devname_from_devnum(st->st_mode, st->st_rdev, ret); } -int device_open_from_devnum(mode_t mode, dev_t devnum, int flags, char **ret); +int device_open_from_devnum(mode_t mode, dev_t devnum, int flags, char **ret_devname); char** device_make_log_fields(sd_device *device); From 8f6ea3e8279c85e75f2fac75b62062d2789e68a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:21:06 +0100 Subject: [PATCH 17/25] sd-journal/catalog: use strdup_to(), rename output param --- src/libsystemd/sd-journal/catalog.c | 29 ++++++----------------------- src/libsystemd/sd-journal/catalog.h | 2 +- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 826d67371a..a0b673f65f 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -223,10 +223,7 @@ static int catalog_entry_lang( const char* deflang, char **ret) { - size_t c; - char *z; - - c = strlen(t); + size_t c = strlen(t); if (c < 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "[%s:%u] Language too short.", filename, line); @@ -243,12 +240,7 @@ static int catalog_entry_lang( log_warning("[%s:%u] language differs from default for file", filename, line); } - z = strdup(t); - if (!z) - return -ENOMEM; - - *ret = z; - return 0; + return strdup_to(ret, t); } int catalog_import_file(OrderedHashmap *h, const char *path) { @@ -588,15 +580,14 @@ static const char *find_id(void *p, sd_id128_t id) { le64toh(f->offset); } -int catalog_get(const char* database, sd_id128_t id, char **_text) { +int catalog_get(const char* database, sd_id128_t id, char **ret_text) { _cleanup_close_ int fd = -EBADF; void *p = NULL; - struct stat st = {}; - char *text = NULL; + struct stat st; int r; const char *s; - assert(_text); + assert(ret_text); r = open_mmap(database, &fd, &st, &p); if (r < 0) @@ -608,15 +599,7 @@ int catalog_get(const char* database, sd_id128_t id, char **_text) { goto finish; } - text = strdup(s); - if (!text) { - r = -ENOMEM; - goto finish; - } - - *_text = text; - r = 0; - + r = strdup_to(ret_text, s); finish: (void) munmap(p, st.st_size); diff --git a/src/libsystemd/sd-journal/catalog.h b/src/libsystemd/sd-journal/catalog.h index df27869fb3..b5a97fa6fb 100644 --- a/src/libsystemd/sd-journal/catalog.h +++ b/src/libsystemd/sd-journal/catalog.h @@ -11,7 +11,7 @@ int catalog_import_file(OrderedHashmap *h, const char *path); int catalog_update(const char* database, const char* root, const char* const* dirs); -int catalog_get(const char* database, sd_id128_t id, char **data); +int catalog_get(const char* database, sd_id128_t id, char **ret_text); int catalog_list(FILE *f, const char* database, bool oneline); int catalog_list_items(FILE *f, const char* database, bool oneline, char **items); int catalog_file_lang(const char *filename, char **lang); From 4ab305fa3b12df5df5667a5d1bb9c8f014608d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:32:55 +0100 Subject: [PATCH 18/25] basic/time-util: use strdup_to() and reformat a bit --- src/basic/time-util.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 822d660fda..b94f37c31c 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -1606,38 +1606,24 @@ bool clock_supported(clockid_t clock) { int get_timezone(char **ret) { _cleanup_free_ char *t = NULL; - const char *e; - char *z; int r; assert(ret); r = readlink_malloc("/etc/localtime", &t); - if (r == -ENOENT) { + if (r == -ENOENT) /* If the symlink does not exist, assume "UTC", like glibc does */ - z = strdup("UTC"); - if (!z) - return -ENOMEM; - - *ret = z; - return 0; - } + return strdup_to(ret, "UTC"); if (r < 0) - return r; /* returns EINVAL if not a symlink */ + return r; /* Return EINVAL if not a symlink */ - e = PATH_STARTSWITH_SET(t, "/usr/share/zoneinfo/", "../usr/share/zoneinfo/"); + const char *e = PATH_STARTSWITH_SET(t, "/usr/share/zoneinfo/", "../usr/share/zoneinfo/"); if (!e) return -EINVAL; - if (!timezone_is_valid(e, LOG_DEBUG)) return -EINVAL; - z = strdup(e); - if (!z) - return -ENOMEM; - - *ret = z; - return 0; + return strdup_to(ret, e); } time_t mktime_or_timegm(struct tm *tm, bool utc) { From 237bbec1173b41c0e0f2eaf9c30e19ab82073b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:34:01 +0100 Subject: [PATCH 19/25] machinectl: use strdup_to() and move cleanup out of the loop It doesn't matter much, but it seems nicer to call va_end() just once. --- src/machine/machinectl.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index bab3719a49..32a60e388f 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -159,24 +159,22 @@ static int call_get_os_release(sd_bus *bus, const char *method, const char *name if (r < 0) return bus_log_parse_error(r); + r = 0; va_start(ap, query); for (count = 0; count < awaited_args; count++) { - char *val, **out; + char **out; out = va_arg(ap, char **); assert(out); if (query_res[count]) { - val = strdup(query_res[count]); - if (!val) { - va_end(ap); - return -ENOMEM; - } - *out = val; + r = strdup_to(out, query_res[count]); + if (r < 0) + break; } } va_end(ap); - return 0; + return r; } static int call_get_addresses( From b41f88a23c1117eec892d212d001c37d1a0aa56d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:34:24 +0100 Subject: [PATCH 20/25] shared/dissect-image: use strdup_to_full() in one more place --- src/shared/dissect-image.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index eac3641abd..ad6c5c065f 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -268,16 +268,8 @@ int probe_filesystem_full( (void) blkid_probe_lookup_value(b, "TYPE", &fstype, NULL); if (fstype) { - char *t; - log_debug("Probed fstype '%s' on partition %s.", fstype, path); - - t = strdup(fstype); - if (!t) - return -ENOMEM; - - *ret_fstype = t; - return 1; + return strdup_to_full(ret_fstype, fstype); } not_found: From 32baca00230a3b9592e7d67e0eee34a04f7b4dcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:34:52 +0100 Subject: [PATCH 21/25] logind: use strdup_to() --- src/login/logind-dbus.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 27d4f72b4c..c342522e3f 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -989,53 +989,41 @@ static int create_session( user->gc_mode = USER_GC_BY_PIN; if (!isempty(tty)) { - session->tty = strdup(tty); - if (!session->tty) { - r = -ENOMEM; + r = strdup_to(&session->tty, tty); + if (r < 0) goto fail; - } session->tty_validity = TTY_FROM_PAM; } if (!isempty(display)) { - session->display = strdup(display); - if (!session->display) { - r = -ENOMEM; + r = strdup_to(&session->display, display); + if (r < 0) goto fail; - } } if (!isempty(remote_user)) { - session->remote_user = strdup(remote_user); - if (!session->remote_user) { - r = -ENOMEM; + r = strdup_to(&session->remote_user, remote_user); + if (r < 0) goto fail; - } } if (!isempty(remote_host)) { - session->remote_host = strdup(remote_host); - if (!session->remote_host) { - r = -ENOMEM; + r = strdup_to(&session->remote_host, remote_host); + if (r < 0) goto fail; - } } if (!isempty(service)) { - session->service = strdup(service); - if (!session->service) { - r = -ENOMEM; + r = strdup_to(&session->service, service); + if (r < 0) goto fail; - } } if (!isempty(desktop)) { - session->desktop = strdup(desktop); - if (!session->desktop) { - r = -ENOMEM; + r = strdup_to(&session->desktop, desktop); + if (r < 0) goto fail; - } } if (seat) { From 196166dbb1e65a4d942170cb3856cb7e6d28c6d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:35:32 +0100 Subject: [PATCH 22/25] basic/unit-name: use strdup_to() in slice_build_parent_slice() The handling of the buffer is not obvious, so add a comment. --- src/basic/unit-name.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index a84b14568d..4e2f77c03d 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -797,10 +797,6 @@ good: } int slice_build_parent_slice(const char *slice, char **ret) { - _cleanup_free_ char *s = NULL; - char *dash; - int r; - assert(slice); assert(ret); @@ -812,18 +808,16 @@ int slice_build_parent_slice(const char *slice, char **ret) { return 0; } - s = strdup(slice); + _cleanup_free_ char *s = strdup(slice); if (!s) return -ENOMEM; - dash = strrchr(s, '-'); - if (dash) - strcpy(dash, ".slice"); - else { - r = free_and_strdup(&s, SPECIAL_ROOT_SLICE); - if (r < 0) - return r; - } + char *dash = strrchr(s, '-'); + if (!dash) + return strdup_to_full(ret, SPECIAL_ROOT_SLICE); + + /* We know that s ended with .slice before truncation, so we have enough space. */ + strcpy(dash, ".slice"); *ret = TAKE_PTR(s); return 1; From b8486f2592d4ec330f66f27857334f65c713ef40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:36:03 +0100 Subject: [PATCH 23/25] oomd: use strdup_to() and RET_GATHER() --- src/oom/oomd-util.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c index 6e6678c33d..69d103e71f 100644 --- a/src/oom/oomd-util.c +++ b/src/oom/oomd-util.c @@ -321,17 +321,16 @@ int oomd_kill_by_pgscan_rate(Hashmap *h, const char *prefix, bool dry_run, char if (r == -ENOMEM) return r; /* Treat oom as a hard error */ if (r < 0) { - if (ret == 0) - ret = r; + RET_GATHER(ret, r); continue; /* Try to find something else to kill */ } dump_until = MAX(dump_until, i + 1); - char *selected = strdup(sorted[i]->path); - if (!selected) - return -ENOMEM; - *ret_selected = selected; + ret = r; + r = strdup_to(ret_selected, sorted[i]->path); + if (r < 0) + return r; break; } @@ -365,17 +364,16 @@ int oomd_kill_by_swap_usage(Hashmap *h, uint64_t threshold_usage, bool dry_run, if (r == -ENOMEM) return r; /* Treat oom as a hard error */ if (r < 0) { - if (ret == 0) - ret = r; + RET_GATHER(ret, r); continue; /* Try to find something else to kill */ } dump_until = MAX(dump_until, i + 1); - char *selected = strdup(sorted[i]->path); - if (!selected) - return -ENOMEM; - *ret_selected = selected; + ret = r; + r = strdup_to(ret_selected, sorted[i]->path); + if (r < 0) + return r; break; } @@ -442,9 +440,9 @@ int oomd_cgroup_context_acquire(const char *path, OomdCGroupContext **ret) { return log_debug_errno(r, "Error converting pgscan value to uint64_t: %m"); } - ctx->path = strdup(empty_to_root(path)); - if (!ctx->path) - return -ENOMEM; + r = strdup_to(&ctx->path, empty_to_root(path)); + if (r < 0) + return r; *ret = TAKE_PTR(ctx); return 0; From 045d94248d75bf8db00af7ee970bf8a5cc934e86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 19 Mar 2024 23:36:37 +0100 Subject: [PATCH 24/25] core: use strdup_to() Those are all pretty straightforward. --- src/core/service.c | 11 +++-------- src/core/unit-printf.c | 43 +++++++++--------------------------------- 2 files changed, 12 insertions(+), 42 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index a9047be1ca..10de379ce2 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -5007,14 +5007,9 @@ int service_determine_exec_selinux_label(Service *s, char **ret) { /* Returns the SELinux label used for execution of the main service binary */ - if (s->exec_context.selinux_context) { /* Prefer the explicitly configured label if there is one */ - char *con = strdup(s->exec_context.selinux_context); - if (!con) - return -ENOMEM; - - *ret = con; - return 0; - } + if (s->exec_context.selinux_context) + /* Prefer the explicitly configured label if there is one */ + return strdup_to(ret, s->exec_context.selinux_context); if (s->exec_context.root_image || s->exec_context.n_extension_images > 0 || diff --git a/src/core/unit-printf.c b/src/core/unit-printf.c index 40267531d1..f25e2e3986 100644 --- a/src/core/unit-printf.c +++ b/src/core/unit-printf.c @@ -91,37 +91,22 @@ static int specifier_cgroup(char specifier, const void *data, const char *root, bad_specifier(u, specifier); - if (crt && crt->cgroup_path) { - char *n; - - n = strdup(crt->cgroup_path); - if (!n) - return -ENOMEM; - - *ret = n; - return 0; - } + if (crt && crt->cgroup_path) + return strdup_to(ret, crt->cgroup_path); return unit_default_cgroup_path(u, ret); } static int specifier_cgroup_root(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const Unit *u = ASSERT_PTR(userdata); - char *n; bad_specifier(u, specifier); - n = strdup(u->manager->cgroup_root); - if (!n) - return -ENOMEM; - - *ret = n; - return 0; + return strdup_to(ret, u->manager->cgroup_root); } static int specifier_cgroup_slice(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const Unit *u = ASSERT_PTR(userdata), *slice; - char *n; bad_specifier(u, specifier); @@ -130,28 +115,18 @@ static int specifier_cgroup_slice(char specifier, const void *data, const char * CGroupRuntime *crt = unit_get_cgroup_runtime(slice); if (crt && crt->cgroup_path) - n = strdup(crt->cgroup_path); - else - return unit_default_cgroup_path(slice, ret); - } else - n = strdup(u->manager->cgroup_root); - if (!n) - return -ENOMEM; + return strdup_to(ret, crt->cgroup_path); - *ret = n; - return 0; + return unit_default_cgroup_path(slice, ret); + } + + return strdup_to(ret, u->manager->cgroup_root); } static int specifier_special_directory(char specifier, const void *data, const char *root, const void *userdata, char **ret) { const Unit *u = ASSERT_PTR(userdata); - char *n; - n = strdup(u->manager->prefix[PTR_TO_UINT(data)]); - if (!n) - return -ENOMEM; - - *ret = n; - return 0; + return strdup_to(ret, u->manager->prefix[PTR_TO_UINT(data)]); } static int specifier_credentials_dir(char specifier, const void *data, const char *root, const void *userdata, char **ret) { From 3a08372d18b0ec9d47f76bd41dbef225d8feeff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 20 Mar 2024 00:11:56 +0100 Subject: [PATCH 25/25] shared/password-quality: inline iterator variable --- src/shared/password-quality-util-pwquality.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shared/password-quality-util-pwquality.c b/src/shared/password-quality-util-pwquality.c index 60816f8245..e070f9636c 100644 --- a/src/shared/password-quality-util-pwquality.c +++ b/src/shared/password-quality-util-pwquality.c @@ -101,7 +101,6 @@ int suggest_passwords(void) { _cleanup_strv_free_erase_ char **suggestions = NULL; _cleanup_(erase_and_freep) char *joined = NULL; char buf[PWQ_MAX_ERROR_MESSAGE_LEN]; - size_t i; int r; r = pwq_allocate_context(&pwq); @@ -115,7 +114,7 @@ int suggest_passwords(void) { if (!suggestions) return log_oom(); - for (i = 0; i < N_SUGGESTIONS; i++) { + for (size_t i = 0; i < N_SUGGESTIONS; i++) { r = sym_pwquality_generate(pwq, 64, suggestions + i); if (r < 0) return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to generate password, ignoring: %s",