diff --git a/src/basic/macro.h b/src/basic/macro.h index fb1ae65d28..68d8b062e8 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -463,4 +463,20 @@ typedef struct { assert_cc(sizeof(dummy_t) == 0); +/* A little helper for subtracting 1 off a pointer in a safe UB-free way. This is intended to be used for for + * loops that count down from a high pointer until some base. A naive loop would implement this like this: + * + * for (p = end-1; p >= base; p--) … + * + * But this is not safe because p before the base is UB in C. With this macro the loop becomes this instead: + * + * for (p = PTR_SUB1(end, base); p; p = PTR_SUB1(p, base)) … + * + * And is free from UB! */ +#define PTR_SUB1(p, base) \ + ({ \ + typeof(p) _q = (p); \ + _q && _q > (base) ? &_q[-1] : NULL; \ + }) + #include "log.h" diff --git a/src/basic/path-util.c b/src/basic/path-util.c index 7e81f06650..0be2b9eec8 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -929,8 +929,9 @@ int path_find_first_component(const char **p, bool accept_dot_dot, const char ** static const char *skip_slash_or_dot_backward(const char *path, const char *q) { assert(path); + assert(!q || q >= path); - for (; q >= path; q--) { + for (; q; q = PTR_SUB1(q, path)) { if (*q == '/') continue; if (q > path && strneq(q - 1, "/.", 2)) @@ -995,7 +996,7 @@ int path_find_last_component(const char *path, bool accept_dot_dot, const char * q = path + strlen(path) - 1; q = skip_slash_or_dot_backward(path, q); - if ((q < path) || /* the root directory */ + if (!q || /* the root directory */ (q == path && *q == '.')) { /* path is "." or "./" */ if (next) *next = path; @@ -1006,10 +1007,10 @@ int path_find_last_component(const char *path, bool accept_dot_dot, const char * last_end = q + 1; - while (q >= path && *q != '/') - q--; + while (q && *q != '/') + q = PTR_SUB1(q, path); - last_begin = q + 1; + last_begin = q ? q + 1 : path; len = last_end - last_begin; if (len > NAME_MAX) @@ -1019,10 +1020,7 @@ int path_find_last_component(const char *path, bool accept_dot_dot, const char * if (next) { q = skip_slash_or_dot_backward(path, q); - if (q < path) - *next = path; - else - *next = q + 1; + *next = q ? q + 1 : path; } if (ret) diff --git a/src/basic/strv.h b/src/basic/strv.h index c89c0f64e2..2a858326c6 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -134,7 +134,7 @@ bool strv_overlap(char * const *a, char * const *b) _pure_; _len > 0 ? h + _len - 1 : NULL; \ }); \ (s = i); \ - i > h ? i-- : (i = NULL)) + i = PTR_SUB1(i, h)) #define STRV_FOREACH_BACKWARDS(s, l) \ _STRV_FOREACH_BACKWARDS(s, l, UNIQ_T(h, UNIQ), UNIQ_T(i, UNIQ)) diff --git a/src/core/namespace.c b/src/core/namespace.c index 2d5bedbbcd..4ede17788b 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -771,22 +771,20 @@ static void drop_nop(MountEntry *m, size_t *n) { /* Only suppress such subtrees for READONLY, READWRITE and READWRITE_IMPLICIT entries */ if (IN_SET(f->mode, READONLY, READWRITE, READWRITE_IMPLICIT)) { - MountEntry *p; - bool found = false; + MountEntry *found = NULL; /* Now let's find the first parent of the entry we are looking at. */ - for (p = t-1; p >= m; p--) { + for (MountEntry *p = PTR_SUB1(t, m); p; p = PTR_SUB1(p, m)) if (path_startswith(mount_entry_path(f), mount_entry_path(p))) { - found = true; + found = p; break; } - } /* We found it, let's see if it's the same mode, if so, we can drop this entry */ - if (found && p->mode == f->mode) { + if (found && found->mode == f->mode) { log_debug("%s (%s) is made redundant by %s (%s)", mount_entry_path(f), mount_mode_to_string(f->mode), - mount_entry_path(p), mount_mode_to_string(p->mode)); + mount_entry_path(found), mount_mode_to_string(found->mode)); mount_entry_done(f); continue; } diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index a58877e11e..f22088db57 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -810,32 +810,26 @@ int device_set_subsystem(sd_device *device, const char *subsystem) { int device_set_drivers_subsystem(sd_device *device) { _cleanup_free_ char *subsystem = NULL; - const char *syspath, *drivers, *p; + const char *devpath, *drivers, *p; int r; assert(device); - r = sd_device_get_syspath(device, &syspath); + r = sd_device_get_devpath(device, &devpath); if (r < 0) return r; - drivers = strstr(syspath, "/drivers/"); + drivers = strstr(devpath, "/drivers/"); if (!drivers) return -EINVAL; - for (p = drivers - 1; p >= syspath; p--) - if (*p == '/') - break; - - if (p <= syspath) - /* syspath does not start with /sys/ ?? */ - return -EINVAL; - p++; - if (p >= drivers) - /* refuse duplicated slashes */ + r = path_find_last_component(devpath, false, &drivers, &p); + if (r < 0) + return r; + if (r == 0) return -EINVAL; - subsystem = strndup(p, drivers - p); + subsystem = strndup(p, r); if (!subsystem) return -ENOMEM; diff --git a/src/libsystemd/sd-device/test-sd-device.c b/src/libsystemd/sd-device/test-sd-device.c index 2d2157ba70..8a534ca8ef 100644 --- a/src/libsystemd/sd-device/test-sd-device.c +++ b/src/libsystemd/sd-device/test-sd-device.c @@ -12,7 +12,8 @@ #include "time-util.h" static void test_sd_device_one(sd_device *d) { - const char *syspath, *subsystem, *val; + _cleanup_(sd_device_unrefp) sd_device *device_from_id = NULL; + const char *syspath, *subsystem, *id, *val; dev_t devnum; usec_t usec; int i, r; @@ -57,7 +58,17 @@ static void test_sd_device_one(sd_device *d) { r = sd_device_get_property_value(d, "ID_NET_DRIVER", &val); assert_se(r >= 0 || r == -ENOENT); - log_info("syspath:%s subsystem:%s initialized:%s", syspath, strna(subsystem), yes_no(i)); + r = device_get_device_id(d, &id); + assert_se(r >= 0); + + r = sd_device_new_from_device_id(&device_from_id, id); + assert_se(r >= 0); + + r = sd_device_get_syspath(device_from_id, &val); + assert_se(r >= 0); + assert_se(streq(syspath, val)); + + log_info("syspath:%s subsystem:%s id:%s initialized:%s", syspath, strna(subsystem), id, yes_no(i)); } TEST(sd_device_enumerator_devices) { diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index d3b63b1f8f..0adc1d58e3 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -152,7 +152,6 @@ struct udev_list_entry *udev_list_get_entry(struct udev_list *list) { else { _cleanup_free_ struct udev_list_entry **buf = NULL; struct udev_list_entry *entry, **p; - size_t j; buf = new(struct udev_list_entry *, n); if (!buf) @@ -164,7 +163,7 @@ struct udev_list_entry *udev_list_get_entry(struct udev_list *list) { typesafe_qsort(buf, n, udev_list_entry_compare_func); - for (j = n; j > 0; j--) + for (size_t j = n; j > 0; j--) LIST_PREPEND(entries, list->entries, buf[j-1]); } diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 8e603f1f38..88ff4ce45e 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -321,24 +321,19 @@ int seat_switch_to_next(Seat *s) { } int seat_switch_to_previous(Seat *s) { - unsigned start, i; - Session *session; - if (MALLOC_ELEMENTSOF(s->positions) == 0) return -EINVAL; - start = 1; - if (s->active && s->active->position > 0) - start = s->active->position; + size_t start = s->active && s->active->position > 0 ? s->active->position : 1; - for (i = start - 1; i > 0; --i) { - session = seat_get_position(s, i); + for (size_t i = start - 1; i > 0; i--) { + Session *session = seat_get_position(s, i); if (session) return session_activate(session); } - for (i = MALLOC_ELEMENTSOF(s->positions) - 1; i > start; --i) { - session = seat_get_position(s, i); + for (size_t i = MALLOC_ELEMENTSOF(s->positions) - 1; i > start; i--) { + Session *session = seat_get_position(s, i); if (session) return session_activate(session); } diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 48395fea24..0ae1de1c5b 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -168,24 +168,18 @@ int dns_label_unescape_suffix(const char *name, const char **label_terminal, cha /* Skip current terminal character (and accept domain names ending it ".") */ if (*terminal == 0) - terminal--; + terminal = PTR_SUB1(terminal, name); if (terminal >= name && *terminal == '.') - terminal--; + terminal = PTR_SUB1(terminal, name); /* Point name to the last label, and terminal to the preceding terminal symbol (or make it a NULL pointer) */ - for (;;) { - if (terminal < name) { - /* Reached the first label, so indicate that there are no more */ - terminal = NULL; - break; - } - + while (terminal) { /* Find the start of the last label */ if (*terminal == '.') { const char *y; unsigned slashes = 0; - for (y = terminal - 1; y >= name && *y == '\\'; y--) + for (y = PTR_SUB1(terminal, name); y && *y == '\\'; y = PTR_SUB1(y, name)) slashes++; if (slashes % 2 == 0) { @@ -198,7 +192,7 @@ int dns_label_unescape_suffix(const char *name, const char **label_terminal, cha } } - terminal--; + terminal = PTR_SUB1(terminal, name); } r = dns_label_unescape(&name, dest, sz, 0); diff --git a/src/test/test-macro.c b/src/test/test-macro.c index 960ff4bc09..ba319953cd 100644 --- a/src/test/test-macro.c +++ b/src/test/test-macro.c @@ -433,4 +433,27 @@ TEST(DECIMAL_STR_MAX) { assert_se(DECIMAL_STR_MAX(uint64_t) == DECIMAL_STR_WIDTH(u64_longest)+1); } +TEST(PTR_SUB1) { + static const uint64_t x[4] = { 2, 3, 4, 5 }; + const uint64_t *p; + + p = x + ELEMENTSOF(x)-1; + assert_se(*p == 5); + + p = PTR_SUB1(p, x); + assert_se(*p == 4); + + p = PTR_SUB1(p, x); + assert_se(*p == 3); + + p = PTR_SUB1(p, x); + assert_se(*p == 2); + + p = PTR_SUB1(p, x); + assert_se(!p); + + p = PTR_SUB1(p, x); + assert_se(!p); +} + DEFINE_TEST_MAIN(LOG_INFO); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index d245c2c7b3..b46cb0f906 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1830,7 +1830,7 @@ static int udev_rule_apply_token_to_event( bool found = false; /* Drop the last line. */ - for (char *p = buf + strlen(buf) - 1; p >= buf; p--) + for (char *p = PTR_SUB1(buf + strlen(buf), buf); p; p = PTR_SUB1(p, buf)) if (strchr(NEWLINE, *p)) { *p = '\0'; found = true;