Merge pull request #23504 from keszybz/bls-reordering

Refactor the BLS and add a description of version sorts

3 files changed, 40 insertions, 12 deletions

diff --git a/src/fundamental/string-util-fundamental.c b/src/fundamental/string-util-fundamental.c
index feccb822ff..169568e244 100644
--- a/src/fundamental/string-util-fundamental.c
+++ b/src/fundamental/string-util-fundamental.c
@@ -124,8 +124,8 @@ sd_int strverscmp_improved(const sd_char *a, const sd_char *b) {
          *  (newer) 124-1
          */
 
-        if (isempty(a) || isempty(b))
-                return CMP(strcmp_ptr(a, b), 0);
+        a = strempty(a);
+        b = strempty(b);
 
         for (;;) {
                 const sd_char *aa, *bb;
@@ -150,7 +150,7 @@ sd_int strverscmp_improved(const sd_char *a, const sd_char *b) {
                 }
 
                 /* If at least one string reaches the end, then longer is newer.
-                 * Note that except for '~' prefixed segments, a string has more segments is newer.
+                 * Note that except for '~' prefixed segments, a string which has more segments is newer.
                  * So, this check must be after the '~' check. */
                 if (*a == '\0' || *b == '\0')
                         return CMP(*a, *b);
@@ -187,12 +187,6 @@ sd_int strverscmp_improved(const sd_char *a, const sd_char *b) {
                 }
 
                 if (is_digit(*a) || is_digit(*b)) {
-                        /* Skip leading '0', to make 00123 equivalent to 123. */
-                        while (*a == '0')
-                                a++;
-                        while (*b == '0')
-                                b++;
-
                         /* Find the leading numeric segments. One may be an empty string. So,
                          * numeric segments are always newer than alpha segments. */
                         for (aa = a; is_digit(*aa); aa++)
@@ -200,6 +194,17 @@ sd_int strverscmp_improved(const sd_char *a, const sd_char *b) {
                         for (bb = b; is_digit(*bb); bb++)
                                 ;
 
+                        /* Check if one of the strings was empty, but the other not. */
+                        r = CMP(a != aa, b != bb);
+                        if (r != 0)
+                                return r;
+
+                        /* Skip leading '0', to make 00123 equivalent to 123. */
+                        while (*a == '0')
+                                a++;
+                        while (*b == '0')
+                                b++;
+
                         /* To compare numeric segments without parsing their values, first compare the
                          * lengths of the segments. Eg. 12345 vs 123, longer is newer. */
                         r = CMP(aa - a, bb - b);
@@ -228,7 +233,7 @@ sd_int strverscmp_improved(const sd_char *a, const sd_char *b) {
                                 return r;
                 }
 
-                /* The current segments are equivalent. Let's compare the next one. */
+                /* The current segments are equivalent. Let's move to the next one. */
                 a = aa;
                 b = bb;
         }
diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c
index 19e00b996e..003dbc0a95 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -120,7 +120,7 @@ static int session_device_open(SessionDevice *sd, bool active) {
         assert(sd->type != DEVICE_TYPE_UNKNOWN);
         assert(sd->node);
 
-        /* open device and try to get an udev_device from it */
+        /* open device and try to get a udev_device from it */
         fd = open(sd->node, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
         if (fd < 0)
                 return -errno;
diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c
index 902d431d9a..1a753b836a 100644
--- a/src/test/test-string-util.c
+++ b/src/test/test-string-util.c
@@ -852,8 +852,8 @@ static void test_strverscmp_improved_newer(const char *older, const char *newer)
 
 TEST(strverscmp_improved) {
         static const char * const versions[] = {
-                "",
                 "~1",
+                "",
                 "ab",
                 "abb",
                 "abc",
@@ -917,6 +917,29 @@ TEST(strverscmp_improved) {
         /* invalid characters */
         assert_se(strverscmp_improved("123_aa2-67.89", "123aa+2-67.89") == 0);
 
+        /* some corner cases */
+        assert_se(strverscmp_improved("123.", "123") > 0);     /* One more version segment */
+        assert_se(strverscmp_improved("12_3", "123") < 0);     /* 12 < 123 */
+        assert_se(strverscmp_improved("12_3", "12") > 0);      /* 3 > '' */
+        assert_se(strverscmp_improved("12_3", "12.3") > 0);    /* 3 > '' */
+        assert_se(strverscmp_improved("123.0", "123") > 0);    /* 0 > '' */
+        assert_se(strverscmp_improved("123_0", "123") > 0);    /* 0 > '' */
+        assert_se(strverscmp_improved("123..0", "123.0") < 0); /* '' < 0 */
+
+        /* empty strings or strings with ignored characters only */
+        assert_se(strverscmp_improved("", NULL) == 0);
+        assert_se(strverscmp_improved(NULL, "") == 0);
+        assert_se(strverscmp_improved("0_", "0") == 0);
+        assert_se(strverscmp_improved("_0_", "0") == 0);
+        assert_se(strverscmp_improved("_0", "0") == 0);
+        assert_se(strverscmp_improved("0", "0___") == 0);
+        assert_se(strverscmp_improved("", "_") == 0);
+        assert_se(strverscmp_improved("_", "") == 0);
+        assert_se(strverscmp_improved("_", "_") == 0);
+        assert_se(strverscmp_improved("", "~") > 0);
+        assert_se(strverscmp_improved("~", "") < 0);
+        assert_se(strverscmp_improved("~", "~") == 0);
+
         /* non-ASCII digits */
         (void) setlocale(LC_NUMERIC, "ar_YE.utf8");
         assert_se(strverscmp_improved("1٠١٢٣٤٥٦٧٨٩", "1") == 0);