Merge pull request #17871 from yuwata/sd-device-issue-17772

sd-device: keep escaped strings in DEVLINK= property

.mkosi/mkosi.arch

@@ -56,3 +56,6 @@ BuildPackages=
 Packages=
         libidn2
         qrencode
+
+[Host]
+QemuHeadless=yes

.mkosi/mkosi.debian

@@ -65,3 +65,6 @@ Packages=
         libqrencode4
         locales
         libidn2-0
+
+[Host]
+QemuHeadless=yes

.mkosi/mkosi.fedora

@@ -80,3 +80,6 @@ Packages=
 
 BuildDirectory=mkosi.builddir
 Cache=mkosi.cache
+
+[Host]
+QemuHeadless=yes

.mkosi/mkosi.opensuse

@@ -76,3 +76,6 @@ Packages=
         libseccomp2
         pam
         util-linux
+
+[Host]
+QemuHeadless=yes

.mkosi/mkosi.ubuntu

@@ -69,3 +69,6 @@ Packages=
         libqrencode4
         locales
         libidn2-0
+
+[Host]
+QemuHeadless=yes

hwdb.d/60-keyboard.hwdb

@@ -1276,6 +1276,16 @@ evdev:atkbd:dmi:bvn*:bvr*:bd*:svnMicro-Star*:pn*A10SC*:*
  KEYBOARD_KEY_f1=f20
  KEYBOARD_KEY_f2=f21
 
+# MSI Modern series
+evdev:atkbd:dmi:bvn*:bvr*:bd*:svnMicro-StarInternational*:pnModern*:*
+ KEYBOARD_KEY_f1=f20                                    # Fn+F5 micmute
+ KEYBOARD_KEY_76=f21                                    # Fn+F4 touchpad, becomes meta+ctrl+toggle
+ KEYBOARD_KEY_91=prog1                                  # Fn+F7 Creation Center, sometime F7
+ KEYBOARD_KEY_f2=prog2                                  # Fn+F12 screen rotation
+ KEYBOARD_KEY_97=unknown                                # lid close
+ KEYBOARD_KEY_98=unknown                                # lid open
+                                                        #Fn+PrntScr sends meta+shif+s
+
 ###########################################################
 # MSI
 ###########################################################

man/systemd-resolved.service.xml

@@ -87,7 +87,7 @@
   <refsect1>
     <title>Synthetic Records</title>
 
-    <para><command>systemd-resolved</command> synthetizes DNS resource records (RRs) for the following
+    <para><command>systemd-resolved</command> synthesizes DNS resource records (RRs) for the following
     cases:</para>
 
     <itemizedlist>
@@ -126,10 +126,10 @@
 
       <listitem><para>Single-label names are resolved using LLMNR on all local interfaces where LLMNR is
       enabled. Lookups for IPv4 addresses are only sent via LLMNR on IPv4, and lookups for IPv6 addresses are
-      only sent via LLMNR on IPv6. Note that lookups for single-label synthetized names are not routed to
+      only sent via LLMNR on IPv6. Note that lookups for single-label synthesized names are not routed to
       LLMNR, MulticastDNS or unicast DNS.</para></listitem>
 
-      <listitem><para>Queries for the address records (A and AAAA) of single-label non-synthetized names are
+      <listitem><para>Queries for the address records (A and AAAA) of single-label non-synthesized names are
       resolved via unicast DNS using search domains. For any interface which defines search domains, such
       look-ups are routed to that interface, suffixed with each of the search domains defined on that
       interface in turn. When global search domains are defined, such look-ups are routed to all interfaces,
@@ -203,7 +203,7 @@
     determined based on the configured DNS domains for a link: if there's a route-only domain other than
     <literal>~.</literal>, it defaults to false, otherwise to true.</para>
 
-    <para>Effectively this means: in order to support single-label non-synthetized names, define appropriate
+    <para>Effectively this means: in order to support single-label non-synthesized names, define appropriate
     search domains. In order to preferably route all DNS queries not explicitly matched by routing domain
     configuration to a specific link, configure a <literal>~.</literal> route-only domain on it. This will
     ensure that other links will not be considered for these queries (unless they too carry such a routing

src/basic/hashmap.c

@@ -1976,3 +1976,38 @@ IteratedCache* iterated_cache_free(IteratedCache *cache) {
 
         return mfree(cache);
 }
+
+int set_strjoin(Set *s, const char *separator, char **ret) {
+        size_t separator_len, allocated = 0, len = 0;
+        _cleanup_free_ char *str = NULL;
+        const char *value;
+        bool first = true;
+
+        assert(ret);
+
+        separator_len = strlen_ptr(separator);
+
+        SET_FOREACH(value, s) {
+                size_t l = strlen_ptr(value);
+
+                if (l == 0)
+                        continue;
+
+                if (!GREEDY_REALLOC(str, allocated, len + l + (first ? 0 : separator_len) + 1))
+                        return -ENOMEM;
+
+                if (separator_len > 0 && !first) {
+                        memcpy(str + len, separator, separator_len);
+                        len += separator_len;
+                }
+
+                memcpy(str + len, value, l);
+                len += l;
+                first = false;
+        }
+        if (str)
+                str[len] = '\0';
+
+        *ret = TAKE_PTR(str);
+        return 0;
+}

src/basic/set.h

@@ -150,3 +150,5 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Set*, set_free_free);
 
 #define _cleanup_set_free_ _cleanup_(set_freep)
 #define _cleanup_set_free_free_ _cleanup_(set_free_freep)
+
+int set_strjoin(Set *s, const char *separator, char **ret);

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

@@ -319,7 +319,10 @@ static int device_amend(sd_device *device, const char *key, const char *value) {
                 for (const char *p = value;;) {
                         _cleanup_free_ char *word = NULL;
 
-                        r = extract_first_word(&p, &word, NULL, 0);
+                        /* udev rules may set escaped strings, and sd-device does not modify the input
+                         * strings. So, it is also necessary to keep the strings received through
+                         * sd-device-monitor. */
+                        r = extract_first_word(&p, &word, NULL, EXTRACT_RETAIN_ESCAPE);
                         if (r < 0)
                                 return r;
                         if (r == 0)

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

@@ -1518,30 +1518,6 @@ _public_ const char *sd_device_get_devlink_next(sd_device *device) {
         return v;
 }
 
-static char *join_string_set(Set *s) {
-        size_t ret_allocated = 0, ret_len;
-        _cleanup_free_ char *ret = NULL;
-        const char *tag;
-
-        if (!GREEDY_REALLOC(ret, ret_allocated, 2))
-                return NULL;
-
-        strcpy(ret, ":");
-        ret_len = 1;
-
-        SET_FOREACH(tag, s) {
-                char *e;
-
-                if (!GREEDY_REALLOC(ret, ret_allocated, ret_len + strlen(tag) + 2))
-                        return NULL;
-
-                e = stpcpy(stpcpy(ret + ret_len, tag), ":");
-                ret_len = e - ret;
-        }
-
-        return TAKE_PTR(ret);
-}
-
 int device_properties_prepare(sd_device *device) {
         int r;
 
@@ -1557,23 +1533,16 @@ int device_properties_prepare(sd_device *device) {
 
         if (device->property_devlinks_outdated) {
                 _cleanup_free_ char *devlinks = NULL;
-                size_t devlinks_allocated = 0, devlinks_len = 0;
-                const char *devlink;
 
-                for (devlink = sd_device_get_devlink_first(device); devlink; devlink = sd_device_get_devlink_next(device)) {
-                        char *e;
-
-                        if (!GREEDY_REALLOC(devlinks, devlinks_allocated, devlinks_len + strlen(devlink) + 2))
-                                return -ENOMEM;
-                        if (devlinks_len > 0)
-                                stpcpy(devlinks + devlinks_len++, " ");
-                        e = stpcpy(devlinks + devlinks_len, devlink);
-                        devlinks_len = e - devlinks;
-                }
+                r = set_strjoin(device->devlinks, " ", &devlinks);
+                if (r < 0)
+                        return r;
 
+                if (!isempty(devlinks)) {
                         r = device_add_property_internal(device, "DEVLINKS", devlinks);
                         if (r < 0)
                                 return r;
+                }
 
                 device->property_devlinks_outdated = false;
         }
@@ -1581,22 +1550,22 @@ int device_properties_prepare(sd_device *device) {
         if (device->property_tags_outdated) {
                 _cleanup_free_ char *tags = NULL;
 
-                tags = join_string_set(device->all_tags);
-                if (!tags)
-                        return -ENOMEM;
+                r = set_strjoin(device->all_tags, ":", &tags);
+                if (r < 0)
+                        return r;
 
-                if (!streq(tags, ":")) {
+                if (!isempty(tags)) {
                         r = device_add_property_internal(device, "TAGS", tags);
                         if (r < 0)
                                 return r;
                 }
 
-                free(tags);
-                tags = join_string_set(device->current_tags);
-                if (!tags)
-                        return -ENOMEM;
+                tags = mfree(tags);
+                r = set_strjoin(device->current_tags, ":", &tags);
+                if (r < 0)
+                        return r;
 
-                if (!streq(tags, ":")) {
+                if (!isempty(tags)) {
                         r = device_add_property_internal(device, "CURRENT_TAGS", tags);
                         if (r < 0)
                                 return r;

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

@@ -1,9 +1,11 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include "device-enumerator-private.h"
+#include "device-internal.h"
 #include "device-private.h"
 #include "device-util.h"
 #include "hashmap.h"
+#include "nulstr-util.h"
 #include "string-util.h"
 #include "tests.h"
 #include "time-util.h"
@@ -161,6 +163,47 @@ static void test_sd_device_enumerator_filter_subsystem(void) {
         assert_se(n_new_dev <= 10);
 }
 
+static void test_sd_device_new_from_nulstr(void) {
+        const char *devlinks =
+                "/dev/disk/by-partuuid/1290d63a-42cc-4c71-b87c-xxxxxxxxxxxx\0"
+                "/dev/disk/by-path/pci-0000:00:0f.0-scsi-0:0:0:0-part3\0"
+                "/dev/disk/by-label/Arch\\x20Linux\0"
+                "/dev/disk/by-uuid/a07b87e5-4af5-4a59-bde9-yyyyyyyyyyyy\0"
+                "/dev/disk/by-partlabel/Arch\\x20Linux\0"
+                "\0";
+
+        _cleanup_(sd_device_unrefp) sd_device *device = NULL, *from_nulstr = NULL;
+        _cleanup_free_ uint8_t *nulstr_copy = NULL;
+        const char *devlink;
+        const uint8_t *nulstr;
+        size_t len;
+
+        log_info("/* %s */", __func__);
+
+        assert_se(sd_device_new_from_syspath(&device, "/sys/class/net/lo") >= 0);
+
+        /* Yeah, of course, setting devlink to the loopback interface is nonsense. But this is just a
+         * test for generating and parsing nulstr. For issue #17772. */
+        NULSTR_FOREACH(devlink, devlinks) {
+                log_device_info(device, "setting devlink: %s", devlink);
+                assert_se(device_add_devlink(device, devlink) >= 0);
+                assert_se(set_contains(device->devlinks, devlink));
+        }
+
+        /* These properties are necessary for device_new_from_nulstr(). See device_verify(). */
+        assert_se(device_add_property_internal(device, "SEQNUM", "1") >= 0);
+        assert_se(device_add_property_internal(device, "ACTION", "change") >= 0);
+
+        assert_se(device_get_properties_nulstr(device, &nulstr, &len) >= 0);
+        assert_se(nulstr_copy = newdup(uint8_t, nulstr, len));
+        assert_se(device_new_from_nulstr(&from_nulstr, nulstr_copy, len) >= 0);
+
+        NULSTR_FOREACH(devlink, devlinks) {
+                log_device_info(from_nulstr, "checking devlink: %s", devlink);
+                assert_se(set_contains(from_nulstr->devlinks, devlink));
+        }
+}
+
 int main(int argc, char **argv) {
         test_setup_logging(LOG_INFO);
 
@@ -168,5 +211,7 @@ int main(int argc, char **argv) {
         test_sd_device_enumerator_subsystems();
         test_sd_device_enumerator_filter_subsystem();
 
+        test_sd_device_new_from_nulstr();
+
         return 0;
 }

src/test/test-set.c

@@ -150,6 +150,56 @@ static void test_set_ensure_consume(void) {
         assert_se(set_size(m) == 2);
 }
 
+static void test_set_strjoin(void) {
+        _cleanup_set_free_ Set *m = NULL;
+        _cleanup_free_ char *joined = NULL;
+
+        assert_se(set_strjoin(m, NULL, &joined) >= 0);
+        assert_se(!joined);
+        assert_se(set_strjoin(m, "", &joined) >= 0);
+        assert_se(!joined);
+        assert_se(set_strjoin(m, " ", &joined) >= 0);
+        assert_se(!joined);
+        assert_se(set_strjoin(m, "xxx", &joined) >= 0);
+        assert_se(!joined);
+
+        assert_se(set_put_strdup(&m, "aaa") == 1);
+
+        assert_se(set_strjoin(m, NULL, &joined) >= 0);
+        assert_se(streq(joined, "aaa"));
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, "", &joined) >= 0);
+        assert_se(streq(joined, "aaa"));
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, " ", &joined) >= 0);
+        assert_se(streq(joined, "aaa"));
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, "xxx", &joined) >= 0);
+        assert_se(streq(joined, "aaa"));
+
+        assert_se(set_put_strdup(&m, "bbb") == 1);
+        assert_se(set_put_strdup(&m, "aaa") == 0);
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, NULL, &joined) >= 0);
+        assert_se(STR_IN_SET(joined, "aaabbb", "bbbaaa"));
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, "", &joined) >= 0);
+        assert_se(STR_IN_SET(joined, "aaabbb", "bbbaaa"));
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, " ", &joined) >= 0);
+        assert_se(STR_IN_SET(joined, "aaa bbb", "bbb aaa"));
+
+        joined = mfree(joined);
+        assert_se(set_strjoin(m, "xxx", &joined) >= 0);
+        assert_se(STR_IN_SET(joined, "aaaxxxbbb", "bbbxxxaaa"));
+}
+
 int main(int argc, const char *argv[]) {
         test_set_steal_first();
         test_set_free_with_destructor();
@@ -160,6 +210,7 @@ int main(int argc, const char *argv[]) {
         test_set_ensure_allocated();
         test_set_ensure_put();
         test_set_ensure_consume();
+        test_set_strjoin();
 
         return 0;
 }

test/units/testsuite-55.sh

@@ -2,14 +2,26 @@
 set -ex
 set -o pipefail
 
+function has_tag_internal() {
+    udevadm info /dev/null | sed -n '/E: '$1'=/ {s/E: '$1'=/:/; s/$/:/; p}' | grep -q ":$2:"
+}
+
+function has_tag() {
+    has_tag_internal TAGS $1
+}
+
+function has_current_tag() {
+    has_tag_internal CURRENT_TAGS $1
+}
+
 mkdir -p /run/udev/rules.d/
 
 ! test -f /run/udev/tags/added/c1:3 &&
     ! test -f /run/udev/tags/changed/c1:3 &&
-    udevadm info /dev/null | grep -q -v 'E: TAGS=.*:added:.*' &&
-    udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:added:.*' &&
-    udevadm info /dev/null | grep -q -v 'E: TAGS=.*:changed:.*' &&
-    udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*'
+    ! has_tag added &&
+    ! has_current_tag added &&
+    ! has_tag changed &&
+    ! has_current_tag changed
 
 cat > /run/udev/rules.d/50-testsuite.rules <<EOF
 ACTION=="add", SUBSYSTEM=="mem", KERNEL=="null", TAG+="added"
@@ -22,10 +34,10 @@ udevadm trigger -c add /dev/null
 while : ; do
     test -f /run/udev/tags/added/c1:3 &&
         ! test -f /run/udev/tags/changed/c1:3 &&
-        udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
-        udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:added:.*' &&
-        udevadm info /dev/null | grep -q -v 'E: TAGS=.*:changed:.*' &&
-        udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*' &&
+        has_tag added &&
+        has_current_tag added &&
+        ! has_tag changed &&
+        ! has_current_tag changed &&
         break
 
     sleep .5
@@ -37,10 +49,10 @@ udevadm trigger -c change /dev/null
 while : ; do
     test -f /run/udev/tags/added/c1:3 &&
         test -f /run/udev/tags/changed/c1:3 &&
-        udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
-        udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:added:.*' &&
-        udevadm info /dev/null | grep -q 'E: TAGS=.*:changed:.*' &&
-        udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:changed:.*' &&
+        has_tag added &&
+        ! has_current_tag added &&
+        has_tag changed &&
+        has_current_tag changed &&
         break
 
     sleep .5
@@ -52,10 +64,10 @@ udevadm trigger -c add /dev/null
 while : ; do
     test -f /run/udev/tags/added/c1:3 &&
         test -f /run/udev/tags/changed/c1:3 &&
-        udevadm info /dev/null | grep -q 'E: TAGS=.*:added:.*' &&
-        udevadm info /dev/null | grep -q 'E: CURRENT_TAGS=.*:added:.*' &&
-        udevadm info /dev/null | grep -q 'E: TAGS=.*:changed:.*' &&
-        udevadm info /dev/null | grep -q -v 'E: CURRENT_TAGS=.*:changed:.*' &&
+        has_tag added &&
+        has_current_tag added &&
+        has_tag changed &&
+        ! has_current_tag changed &&
         break
 
     sleep .5