util-lib: introduce new empty_or_root() helper (#8746)
authorLennart Poettering <lennart@poettering.net>
Wed, 18 Apr 2018 12:20:49 +0000 (14:20 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Wed, 18 Apr 2018 12:20:49 +0000 (14:20 +0200)
We check the same condition at various places. Let's add a trivial,
common helper for this, and use it everywhere.

It's not going to make things much faster or much shorter, but I think a
lot more readable

coccinelle/empty-or-root.cocci [new file with mode: 0644]
src/basic/cgroup-util.c
src/basic/fs-util.c
src/basic/path-util.c
src/basic/path-util.h
src/basic/unit-name.c
src/basic/user-util.c
src/cgtop/cgtop.c
src/core/cgroup.c
src/shared/path-lookup.c
src/test/test-path-util.c

diff --git a/coccinelle/empty-or-root.cocci b/coccinelle/empty-or-root.cocci
new file mode 100644 (file)
index 0000000..bf2f614
--- /dev/null
@@ -0,0 +1,10 @@
+@@
+expression s;
+@@
+- (isempty(s) || path_equal(s, "/"))
++ empty_or_root(s)
+@@
+expression s;
+@@
+- (!isempty(s) && !path_equal(s, "/"))
++ !empty_or_root(s)
index 46ae36d..b5b0d58 100644 (file)
@@ -421,7 +421,7 @@ int cg_migrate(
                          * exist in the root cgroup, we only check for
                          * them there. */
                         if (cfrom &&
-                            (isempty(pfrom) || path_equal(pfrom, "/")) &&
+                            empty_or_root(pfrom) &&
                             is_kernel_thread(pid) > 0)
                                 continue;
 
@@ -1185,7 +1185,7 @@ int cg_is_empty_recursive(const char *controller, const char *path) {
         assert(path);
 
         /* The root cgroup is always populated */
-        if (controller && (isempty(path) || path_equal(path, "/")))
+        if (controller && empty_or_root(path))
                 return false;
 
         r = cg_unified_controller(controller);
index bfdd928..8ce4571 100644 (file)
@@ -564,10 +564,6 @@ int inotify_add_watch_fd(int fd, int what, uint32_t mask) {
         return r;
 }
 
-static bool noop_root(const char *root) {
-        return isempty(root) || path_equal(root, "/");
-}
-
 static bool safe_transition(const struct stat *a, const struct stat *b) {
         /* Returns true if the transition from a to b is safe, i.e. that we never transition from unprivileged to
          * privileged files or directories. Why bother? So that unprivileged code can't symlink to privileged files
@@ -618,7 +614,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
          * specified path. */
 
         /* A root directory of "/" or "" is identical to none */
-        if (noop_root(original_root))
+        if (empty_or_root(original_root))
                 original_root = NULL;
 
         if (!original_root && !ret && (flags & (CHASE_NONEXISTENT|CHASE_NO_AUTOFS|CHASE_SAFE|CHASE_OPEN)) == CHASE_OPEN) {
@@ -703,7 +699,7 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
 
                         /* If we already are at the top, then going up will not change anything. This is in-line with
                          * how the kernel handles this. */
-                        if (isempty(done) || path_equal(done, "/"))
+                        if (empty_or_root(done))
                                 continue;
 
                         parent = dirname_malloc(done);
@@ -889,7 +885,7 @@ int chase_symlinks_and_open(
         if (chase_flags & CHASE_NONEXISTENT)
                 return -EINVAL;
 
-        if (noop_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) {
+        if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) {
                 /* Shortcut this call if none of the special features of this call are requested */
                 r = open(path, open_flags);
                 if (r < 0)
@@ -929,7 +925,7 @@ int chase_symlinks_and_opendir(
         if (chase_flags & CHASE_NONEXISTENT)
                 return -EINVAL;
 
-        if (noop_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) {
+        if (empty_or_root(root) && !ret_path && (chase_flags & (CHASE_NO_AUTOFS|CHASE_SAFE)) == 0) {
                 /* Shortcut this call if none of the special features of this call are requested */
                 d = opendir(path);
                 if (!d)
index 5572c21..2ecb143 100644 (file)
@@ -640,7 +640,7 @@ char *prefix_root(const char *root, const char *path) {
         while (path[0] == '/' && path[1] == '/')
                 path++;
 
-        if (isempty(root) || path_equal(root, "/"))
+        if (empty_or_root(root))
                 return strdup(path);
 
         l = strlen(root) + 1 + strlen(path) + 1;
@@ -965,3 +965,14 @@ bool dot_or_dot_dot(const char *path) {
 
         return path[2] == 0;
 }
+
+bool empty_or_root(const char *root) {
+
+        /* For operations relative to some root directory, returns true if the specified root directory is redundant,
+         * i.e. either / or NULL or the empty string or any equivalent. */
+
+        if (!root)
+                return true;
+
+        return root[strspn(root, "/")] == 0;
+}
index 7ec0a3e..fa8f8e4 100644 (file)
@@ -111,7 +111,7 @@ char *prefix_root(const char *root, const char *path);
                 size_t _l;                                              \
                 while (_path[0] == '/' && _path[1] == '/')              \
                         _path ++;                                       \
-                if (isempty(_root) || path_equal(_root, "/"))           \
+                if (empty_or_root(_root))                               \
                         _ret = _path;                                   \
                 else {                                                  \
                         _l = strlen(_root) + 1 + strlen(_path) + 1;     \
@@ -155,3 +155,5 @@ static inline const char *skip_dev_prefix(const char *p) {
 
         return e ?: p;
 }
+
+bool empty_or_root(const char *root);
index 13653d9..407982e 100644 (file)
@@ -385,7 +385,7 @@ int unit_name_path_escape(const char *f, char **ret) {
 
         path_kill_slashes(p);
 
-        if (STR_IN_SET(p, "/", ""))
+        if (empty_or_root(p))
                 s = strdup("-");
         else {
                 if (!path_is_normalized(p))
index 66a01b0..1effd55 100644 (file)
@@ -221,8 +221,7 @@ int get_user_creds_clean(
             (isempty(*shell) || is_nologin_shell(*shell)))
                 *shell = NULL;
 
-        if (home &&
-            (isempty(*home) || path_equal(*home, "/")))
+        if (home && empty_or_root(*home))
                 *home = NULL;
 
         return 0;
index 7fe812d..de5083b 100644 (file)
@@ -134,7 +134,7 @@ static bool is_root_cgroup(const char *path) {
         if (detect_container() > 0)
                 return false;
 
-        return isempty(path) || path_equal(path, "/");
+        return empty_or_root(path);
 }
 
 static int process(
index b6d185b..1308bc1 100644 (file)
@@ -40,7 +40,7 @@ bool manager_owns_root_cgroup(Manager *m) {
         if (detect_container() > 0)
                 return false;
 
-        return isempty(m->cgroup_root) || path_equal(m->cgroup_root, "/");
+        return empty_or_root(m->cgroup_root);
 }
 
 bool unit_has_root_cgroup(Unit *u) {
index c4b5075..33e38f8 100644 (file)
@@ -484,7 +484,7 @@ int lookup_paths_init(
         assert(scope >= 0);
         assert(scope < _UNIT_FILE_SCOPE_MAX);
 
-        if (!isempty(root_dir) && !path_equal(root_dir, "/")) {
+        if (!empty_or_root(root_dir)) {
                 if (scope == UNIT_FILE_USER)
                         return -EINVAL;
 
index f1a2df3..c65b0b0 100644 (file)
@@ -474,6 +474,19 @@ static void test_skip_dev_prefix(void) {
         assert_se(streq(skip_dev_prefix("foo"), "foo"));
 }
 
+static void test_empty_or_root(void) {
+        assert_se(empty_or_root(NULL));
+        assert_se(empty_or_root(""));
+        assert_se(empty_or_root("/"));
+        assert_se(empty_or_root("//"));
+        assert_se(empty_or_root("///"));
+        assert_se(empty_or_root("/////////////////"));
+        assert_se(!empty_or_root("xxx"));
+        assert_se(!empty_or_root("/xxx"));
+        assert_se(!empty_or_root("/xxx/"));
+        assert_se(!empty_or_root("//yy//"));
+}
+
 int main(int argc, char **argv) {
         log_set_max_level(LOG_DEBUG);
         log_parse_environment();
@@ -494,6 +507,7 @@ int main(int argc, char **argv) {
         test_filename_is_valid();
         test_hidden_or_backup_file();
         test_skip_dev_prefix();
+        test_empty_or_root();
 
         test_systemd_installation_has_version(argv[1]); /* NULL is OK */