Merge pull request #23124 from yuwata/fixes-for-post-merge-review

Fixes for post merge review

src/basic/fs-util.c

@@ -155,7 +155,7 @@ int readlink_malloc(const char *p, char **ret) {
 }
 
 int readlink_value(const char *p, char **ret) {
-        _cleanup_free_ char *link = NULL;
+        _cleanup_free_ char *link = NULL, *name = NULL;
         int r;
 
         assert(p);
@@ -165,7 +165,14 @@ int readlink_value(const char *p, char **ret) {
         if (r < 0)
                 return r;
 
-        return path_extract_filename(link, ret);
+        r = path_extract_filename(link, &name);
+        if (r < 0)
+                return r;
+        if (r == O_DIRECTORY)
+                return -EINVAL;
+
+        *ret = TAKE_PTR(name);
+        return 0;
 }
 
 int readlink_and_make_absolute(const char *p, char **r) {

src/basic/string-util.c

@@ -1162,3 +1162,30 @@ bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok) {
 
         return in_charset(s1, ok) && in_charset(s2, ok);
 }
+
+char *string_replace_char(char *str, char old_char, char new_char) {
+        assert(str);
+        assert(old_char != '\0');
+        assert(new_char != '\0');
+        assert(old_char != new_char);
+
+        for (char *p = strchr(str, old_char); p; p = strchr(p + 1, old_char))
+                *p = new_char;
+
+        return str;
+}
+
+size_t strspn_from_end(const char *str, const char *accept) {
+        size_t n = 0;
+
+        if (isempty(str))
+                return 0;
+
+        if (isempty(accept))
+                return 0;
+
+        for (const char *p = str + strlen(str); p > str && strchr(accept, p[-1]); p--)
+                n++;
+
+        return n;
+}

src/basic/string-util.h

@@ -233,3 +233,7 @@ static inline int string_contains_word(const char *string, const char *separator
 }
 
 bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok);
+
+char *string_replace_char(char *str, char old_char, char new_char);
+
+size_t strspn_from_end(const char *str, const char *accept);

src/libsystemd/sd-device/sd-device.c

@@ -1173,7 +1173,7 @@ _public_ int sd_device_get_devname(sd_device *device, const char **devname) {
 
 static int device_set_sysname_and_sysnum(sd_device *device) {
         _cleanup_free_ char *sysname = NULL;
-        char *p;
+        size_t len, n;
         int r;
 
         assert(device);
@@ -1181,16 +1181,19 @@ static int device_set_sysname_and_sysnum(sd_device *device) {
         r = path_extract_filename(device->devpath, &sysname);
         if (r < 0)
                 return r;
+        if (r == O_DIRECTORY)
+                return -EINVAL;
 
         /* some devices have '!' in their name, change that to '/' */
-        for (p = strchrnul(sysname, '!'); *p != '\0'; p = strchrnul(p, '!'))
-                *p = '/';
+        string_replace_char(sysname, '!', '/');
 
-        /* trailing number (refuse number only sysname)*/
-        for (; p > sysname && isdigit(p[-1]); p--)
-                ;
+        n = strspn_from_end(sysname, DIGITS);
+        len = strlen(sysname);
+        assert(n <= len);
+        if (n == len)
+                n = 0; /* Do not set sysnum for number only sysname. */
 
-        device->sysnum = p > sysname && *p != '\0' ? p : NULL;
+        device->sysnum = n > 0 ? sysname + len - n : NULL;
         return free_and_replace(device->sysname, sysname);
 }
 
@@ -1457,6 +1460,8 @@ int device_get_device_id(sd_device *device, const char **ret) {
                         r = path_extract_filename(device->devpath, &sysname);
                         if (r < 0)
                                 return r;
+                        if (r == O_DIRECTORY)
+                                return -EINVAL;
 
                         if (streq(subsystem, "drivers")) {
                                 /* the 'drivers' pseudo-subsystem is special, and needs the real

src/shared/efi-api.c

@@ -546,11 +546,3 @@ bool efi_has_tpm2(void) {
 }
 
 #endif
-
-char *efi_tilt_backslashes(char *s) {
-        for (char *p = s; *p; p++)
-                if (*p == '\\')
-                        *p = '/';
-
-        return s;
-}

src/shared/efi-api.h

@@ -3,6 +3,7 @@
 
 #include "efivars-fundamental.h"
 #include "efivars.h"
+#include "string-util.h"
 
 /* Various calls for interfacing with EFI variables from the official UEFI specs. */
 
@@ -65,4 +66,6 @@ static inline bool efi_has_tpm2(void) {
 
 #endif
 
-char *efi_tilt_backslashes(char *s);
+static inline char *efi_tilt_backslashes(char *s) {
+        return string_replace_char(s, '\\', '/');
+}

src/shared/firewall-util-nft.c

@@ -31,7 +31,7 @@
 
 static int nfnl_netlink_sendv(
                 sd_netlink *nfnl,
-                sd_netlink_message **messages,
+                sd_netlink_message *messages[static 1],
                 size_t msgcount) {
 
         _cleanup_free_ uint32_t *serial = NULL;

src/test/test-string-util.c

@@ -935,6 +935,26 @@ TEST(strextendf) {
         assert_se(streq(p, "<77>,<99>,<                                                                              88>,<00001234>"));
 }
 
+TEST(string_replace_char) {
+        assert_se(streq(string_replace_char(strdupa(""), 'a', 'b'), ""));
+        assert_se(streq(string_replace_char(strdupa("abc"), 'a', 'b'), "bbc"));
+        assert_se(streq(string_replace_char(strdupa("hoge"), 'a', 'b'), "hoge"));
+        assert_se(streq(string_replace_char(strdupa("aaaa"), 'a', 'b'), "bbbb"));
+        assert_se(streq(string_replace_char(strdupa("aaaa"), 'a', '\t'), "\t\t\t\t"));
+}
+
+TEST(strspn_from_end) {
+        assert_se(strspn_from_end(NULL, NULL) == 0);
+        assert_se(strspn_from_end("hoge", NULL) == 0);
+        assert_se(strspn_from_end(NULL, DIGITS) == 0);
+        assert_se(strspn_from_end("", DIGITS) == 0);
+        assert_se(strspn_from_end("hoge", DIGITS) == 0);
+        assert_se(strspn_from_end("1234", DIGITS) == 4);
+        assert_se(strspn_from_end("aaa1234", DIGITS) == 4);
+        assert_se(strspn_from_end("aaa1234aaa", DIGITS) == 0);
+        assert_se(strspn_from_end("aaa12aa34", DIGITS) == 2);
+}
+
 TEST(streq_skip_trailing_chars) {
         /* NULL is WHITESPACE by default*/
         assert_se(streq_skip_trailing_chars("foo bar", "foo bar", NULL));