[systemd-commits] 2 commits - src/core src/shared src/test

Michal Schmidt michich at kemper.freedesktop.org
Mon Mar 16 14:21:45 PDT 2015


 src/core/namespace.c      |   12 ++++--------
 src/shared/path-util.c    |   37 +++++++++++++++++++++++++++----------
 src/shared/path-util.h    |    1 +
 src/test/test-path-util.c |   36 +++++++++++++++++++++++++-----------
 4 files changed, 57 insertions(+), 29 deletions(-)

New commits:
commit a0827e2b123010c46cfe4f03eebba57d92f9efc4
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Mon Mar 16 22:04:21 2015 +0100

    core/namespace: fix path sorting
    
    The comparison function we use for qsorting paths is overly indifferent.
    Consider these 3 paths for sorting:
     /foo
     /bar
     /foo/foo
    qsort() may compare:
     "/foo" with "/bar" => 0, indifference
     "/bar" with "/foo/foo" => 0, indifference
    and assume transitively that "/foo" and "/foo/foo" are also indifferent.
    
    But this is wrong, we want "/foo" sorted before "/foo/foo".
    The comparison function must be transitive.
    
    Use path_compare(), which behaves properly.
    
    Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1184016

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 4a412a2..f8a2bbc 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -83,9 +83,11 @@ static int append_mounts(BindMount **p, char **strv, MountMode mode) {
 
 static int mount_path_compare(const void *a, const void *b) {
         const BindMount *p = a, *q = b;
+        int d;
 
-        if (path_equal(p->path, q->path)) {
+        d = path_compare(p->path, q->path);
 
+        if (!d) {
                 /* If the paths are equal, check the mode */
                 if (p->mode < q->mode)
                         return -1;
@@ -97,13 +99,7 @@ static int mount_path_compare(const void *a, const void *b) {
         }
 
         /* If the paths are not equal, then order prefixes first */
-        if (path_startswith(p->path, q->path))
-                return 1;
-
-        if (path_startswith(q->path, p->path))
-                return -1;
-
-        return 0;
+        return d;
 }
 
 static void drop_duplicates(BindMount *m, unsigned *n) {

commit 2230852bd9755e1b7bfd1260082471f559b0a005
Author: Michal Schmidt <mschmidt at redhat.com>
Date:   Mon Mar 16 21:58:35 2015 +0100

    shared: add path_compare(), an ordering path comparison
    
    ... and make path_equal() a simple wrapper around it.

diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index cdc61b1..53c0079 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -400,12 +400,18 @@ char* path_startswith(const char *path, const char *prefix) {
         }
 }
 
-bool path_equal(const char *a, const char *b) {
+int path_compare(const char *a, const char *b) {
+        int d;
+
         assert(a);
         assert(b);
 
-        if ((a[0] == '/') != (b[0] == '/'))
-                return false;
+        /* A relative path and an abolute path must not compare as equal.
+         * Which one is sorted before the other does not really matter.
+         * Here a relative path is ordered before an absolute path. */
+        d = (a[0] == '/') - (b[0] == '/');
+        if (d)
+                return d;
 
         for (;;) {
                 size_t j, k;
@@ -414,25 +420,36 @@ bool path_equal(const char *a, const char *b) {
                 b += strspn(b, "/");
 
                 if (*a == 0 && *b == 0)
-                        return true;
+                        return 0;
 
-                if (*a == 0 || *b == 0)
-                        return false;
+                /* Order prefixes first: "/foo" before "/foo/bar" */
+                if (*a == 0)
+                        return -1;
+                if (*b == 0)
+                        return 1;
 
                 j = strcspn(a, "/");
                 k = strcspn(b, "/");
 
-                if (j != k)
-                        return false;
+                /* Alphabetical sort: "/foo/aaa" before "/foo/b" */
+                d = memcmp(a, b, MIN(j, k));
+                if (d)
+                        return (d > 0) - (d < 0); /* sign of d */
 
-                if (memcmp(a, b, j) != 0)
-                        return false;
+                /* Sort "/foo/a" before "/foo/aaa" */
+                d = (j > k) - (j < k);  /* sign of (j - k) */
+                if (d)
+                        return d;
 
                 a += j;
                 b += k;
         }
 }
 
+bool path_equal(const char *a, const char *b) {
+        return path_compare(a, b) == 0;
+}
+
 bool path_equal_or_files_same(const char *a, const char *b) {
         return path_equal(a, b) || files_same(a, b) > 0;
 }
diff --git a/src/shared/path-util.h b/src/shared/path-util.h
index bcf116e..ca81b49 100644
--- a/src/shared/path-util.h
+++ b/src/shared/path-util.h
@@ -44,6 +44,7 @@ char* path_make_absolute_cwd(const char *p);
 int path_make_relative(const char *from_dir, const char *to_path, char **_r);
 char* path_kill_slashes(char *path);
 char* path_startswith(const char *path, const char *prefix) _pure_;
+int path_compare(const char *a, const char *b) _pure_;
 bool path_equal(const char *a, const char *b) _pure_;
 bool path_equal_or_files_same(const char *a, const char *b);
 char* path_join(const char *root, const char *path, const char *rest);
diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 11aa52a..6396fcb 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -27,23 +27,37 @@
 #include "macro.h"
 #include "strv.h"
 
+#define test_path_compare(a, b, result) {                 \
+                assert_se(path_compare(a, b) == result);  \
+                assert_se(path_compare(b, a) == -result); \
+                assert_se(path_equal(a, b) == !result);   \
+                assert_se(path_equal(b, a) == !result);   \
+        }
 
 static void test_path(void) {
-        assert_se(path_equal("/goo", "/goo"));
-        assert_se(path_equal("//goo", "/goo"));
-        assert_se(path_equal("//goo/////", "/goo"));
-        assert_se(path_equal("goo/////", "goo"));
+        test_path_compare("/goo", "/goo", 0);
+        test_path_compare("/goo", "/goo", 0);
+        test_path_compare("//goo", "/goo", 0);
+        test_path_compare("//goo/////", "/goo", 0);
+        test_path_compare("goo/////", "goo", 0);
+
+        test_path_compare("/goo/boo", "/goo//boo", 0);
+        test_path_compare("//goo/boo", "/goo/boo//", 0);
 
-        assert_se(path_equal("/goo/boo", "/goo//boo"));
-        assert_se(path_equal("//goo/boo", "/goo/boo//"));
+        test_path_compare("/", "///", 0);
 
-        assert_se(path_equal("/", "///"));
+        test_path_compare("/x", "x/", 1);
+        test_path_compare("x/", "/", -1);
 
-        assert_se(!path_equal("/x", "x/"));
-        assert_se(!path_equal("x/", "/"));
+        test_path_compare("/x/./y", "x/y", 1);
+        test_path_compare("x/.y", "x/y", -1);
 
-        assert_se(!path_equal("/x/./y", "x/y"));
-        assert_se(!path_equal("x/.y", "x/y"));
+        test_path_compare("foo", "/foo", -1);
+        test_path_compare("/foo", "/foo/bar", -1);
+        test_path_compare("/foo/aaa", "/foo/b", -1);
+        test_path_compare("/foo/aaa", "/foo/b/a", -1);
+        test_path_compare("/foo/a", "/foo/aaa", -1);
+        test_path_compare("/foo/a/b", "/foo/aaa", -1);
 
         assert_se(path_is_absolute("/"));
         assert_se(!path_is_absolute("./"));



More information about the systemd-commits mailing list