UCS/VFS: Improve symlink paths and handling of duplicate objects (#10916)

Author: yosefe

src/tools/vfs/vfs_main.c

@@ -277,10 +277,12 @@ static int vfs_unlink_socket(int silent_notexist)
 }
 
 /* return 0 or the (negative) value of errno in case of error */
-static int vfs_listen(int silent_addinuse_err)
+static int vfs_listen(int silent_addrinuse_err)
 {
     int listen_fd, ret;
 
+    vfs_log("listening on socket %s", (const char*)g_sockaddr.sun_path);
+
     ret = umask(~S_IRWXU);
     if (ret < 0) {
         ret = -errno;
@@ -304,7 +306,7 @@ static int vfs_listen(int silent_addinuse_err)
                sizeof(g_sockaddr));
     if (ret < 0) {
         ret = -errno;
-        if ((errno != EADDRINUSE) || !silent_addinuse_err) {
+        if ((errno != EADDRINUSE) || !silent_addrinuse_err) {
             vfs_error("bind(%s) failed: %m", g_sockaddr.sun_path);
         }
         goto out_close;

src/ucs/sys/string.c

@@ -353,8 +353,7 @@ size_t ucs_string_common_prefix_len(const char *str1, const char *str2)
     return (p1 - str1);
 }
 
-static size_t
-ucs_path_common_parent_length(const char *path1, const char *path2)
+size_t ucs_path_common_parent_length(const char *path1, const char *path2)
 {
     size_t offset, parent_length;
 

src/ucs/sys/string.h

@@ -270,6 +270,18 @@ size_t ucs_string_count_char(const char *str, char c);
 size_t ucs_string_common_prefix_len(const char *str1, const char *str2);
 
 
+/*
+ * Return the length of the common parent directory, without the trailing slash.
+ * For example, for /sys/ab12 and /sys/ab23 return 4.
+ *
+ * @param  path1        String pointing to first path.
+ * @param  path2        String pointing to second path.
+ *
+ * @return length of the common parent directory
+ */
+size_t ucs_path_common_parent_length(const char *path1, const char *path2);
+
+
 /*
  * Return the common parent directory, without the trailing slash.
  * For example, for /sys/ab12 and /sys/ab23 return /sys
@@ -279,7 +291,7 @@ size_t ucs_string_common_prefix_len(const char *str1, const char *str2);
  * @param  common_path  Buffer to hold the common parent path. The size of the
  *                      buffer must be large enough to hold the longest common
  *                      parent path, plus a '\0' terminator.
- * */
+ */
 void ucs_path_get_common_parent(const char *path1, const char *path2,
                                 char *common_path);
 

src/ucs/vfs/base/vfs_obj.c

@@ -91,8 +91,9 @@ struct {
     { \
         int khret; \
         khiter_t khiter = kh_put(_name, _h, _k, &khret); \
-        ucs_assert((khret == UCS_KH_PUT_BUCKET_EMPTY) || \
-                   (khret == UCS_KH_PUT_BUCKET_CLEAR)); \
+        ucs_assertv((khret == UCS_KH_PUT_BUCKET_EMPTY) || \
+                    (khret == UCS_KH_PUT_BUCKET_CLEAR), \
+                    "khret=%d path=%s", khret, (_node)->path); \
         kh_val(_h, khiter) = _node; \
     }
 
@@ -169,31 +170,43 @@ static ucs_vfs_node_t *ucs_vfs_node_create(ucs_vfs_node_t *parent_node,
                                            const char *path,
                                            ucs_vfs_node_type_t type, void *obj)
 {
-    ucs_vfs_node_t *node;
+    ucs_vfs_node_t *node, *existing_node;
 
     node = ucs_malloc(sizeof(*node) + strlen(path) + 1, "vfs_node");
     if (node == NULL) {
-        ucs_error("Failed to allocate vfs_node");
-        return NULL;
+        ucs_error("failed to allocate vfs_node");
+        goto err;
     }
 
-    /* initialize node */
+    /* Initialize node */
     ucs_vfs_node_init(node, type, obj, parent_node);
     strcpy(node->path, path);
 
-    /* add to parent */
-    ucs_list_add_head(&parent_node->children, &node->list);
-
-    /* add to obj hash */
+    /* Add to object hash */
     if (node->obj != NULL) {
+        existing_node = ucs_vfs_node_find_by_obj(node->obj);
+        if (existing_node != NULL) {
+            ucs_error("cannot add vfs node '%s', object %p (%s) already exists",
+                      node->path, node->obj, existing_node->path);
+            goto err_free;
+        }
+
         ucs_vfs_kh_put(vfs_obj, &ucs_vfs_obj_context.obj_hash,
                        (uintptr_t)node->obj, node);
     }
 
     /* add to path hash */
     ucs_vfs_kh_put(vfs_path, &ucs_vfs_obj_context.path_hash, node->path, node);
 
+    /* add to parent */
+    ucs_list_add_head(&parent_node->children, &node->list);
+
     return node;
+
+err_free:
+    ucs_free(node);
+err:
+    return NULL;
 }
 
 /* must be called with lock held */
@@ -512,18 +525,22 @@ static void ucs_vfs_path_list_dir_cb(ucs_vfs_node_t *node,
 static void
 ucs_vfs_get_link_path(ucs_vfs_node_t *node, ucs_string_buffer_t *strb)
 {
-    size_t i, n;
+    size_t i, n, common_length;
 
     ucs_assert(ucs_vfs_check_node(node, UCS_VFS_NODE_TYPE_SYM_LINK));
+    ucs_assertv(node->target != NULL, "node=%p node->path=%s", node,
+                node->path);
 
-    n = ucs_string_count_char(node->path, '/');
+    common_length = ucs_path_common_parent_length(node->path,
+                                                  node->target->path);
+
+    n = ucs_string_count_char(&node->path[common_length], '/');
     for (i = 1; i < n; ++i) {
         ucs_string_buffer_appendf(strb, "../");
     }
 
-    if (node->target != NULL) {
-        ucs_string_buffer_appendf(strb, "%s", &node->target->path[1]);
-    }
+    ucs_string_buffer_appendf(strb, "%s",
+                              &node->target->path[common_length + 1]);
 }
 
 ucs_status_t
@@ -635,7 +652,6 @@ ucs_vfs_obj_add_sym_link(void *obj, void *target_obj, const char *rel_path, ...)
 
 out:
     ucs_spin_unlock(&ucs_vfs_obj_context.lock);
-
     return status;
 }
 

test/gtest/ucs/test_vfs.cc

@@ -285,25 +285,35 @@ UCS_TEST_F(test_vfs_obj, check_ret) {
 
 UCS_MT_TEST_F(test_vfs_obj, add_sym_link, 4)
 {
-    static const char path_to_target[] = "target";
+    static char parent_obj;
+    ucs_vfs_obj_add_dir(NULL, &parent_obj, "parent");
 
-    static char target;
-    ucs_vfs_obj_add_dir(NULL, &target, "target");
-    ucs_vfs_obj_add_sym_link(NULL, &target, "link");
+    // Add target directory and subdirectory
+    // NOTE: Currently links to files are not supported
+    static long target_obj = 0;
+    ucs_vfs_obj_add_dir(&parent_obj, &target_obj, "target");
+
+    // Add source directory and link
+    static char source_dir_obj;
+    ucs_vfs_obj_add_dir(&parent_obj, &source_dir_obj, "source_dir");
+    ucs_vfs_obj_add_sym_link(&source_dir_obj, &target_obj, "link");
+
+    const char *link_path = "/parent/source_dir/link";
+    const char *path_to_target = "../target";
 
     ucs_vfs_path_info_t path_info;
-    EXPECT_UCS_OK(ucs_vfs_path_get_info("/link", &path_info));
+    EXPECT_UCS_OK(ucs_vfs_path_get_info(link_path, &path_info));
     EXPECT_EQ(strlen(path_to_target), path_info.size);
     EXPECT_TRUE(path_info.mode & S_IFLNK);
 
     ucs_string_buffer_t strb;
     ucs_string_buffer_init(&strb);
-    EXPECT_UCS_OK(ucs_vfs_path_get_link("/link", &strb));
+    EXPECT_UCS_OK(ucs_vfs_path_get_link(link_path, &strb));
     EXPECT_STREQ(path_to_target, ucs_string_buffer_cstr(&strb));
     ucs_string_buffer_cleanup(&strb);
 
     barrier();
-    ucs_vfs_obj_remove(&target);
+    ucs_vfs_obj_remove(&parent_obj);
 }
 
 UCS_TEST_F(test_vfs_obj, add_rw_check_ret) {