Skip to content

Commit c1fa6ec

Browse files
yosefeyosefe
authored
UCS/VFS: Improve symlink paths and handling of duplicate objects (openucx#10916)
1 parent e75960c commit c1fa6ec

File tree

5 files changed

+67
-28
lines changed

5 files changed

+67
-28
lines changed

‎src/tools/vfs/vfs_main.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,10 +277,12 @@ static int vfs_unlink_socket(int silent_notexist)
277277
}
278278

279279
/* return 0 or the (negative) value of errno in case of error */
280-
static int vfs_listen(int silent_addinuse_err)
280+
static int vfs_listen(int silent_addrinuse_err)
281281
{
282282
int listen_fd, ret;
283283

284+
vfs_log("listening on socket %s", (const char*)g_sockaddr.sun_path);
285+
284286
ret = umask(~S_IRWXU);
285287
if (ret < 0) {
286288
ret = -errno;
@@ -304,7 +306,7 @@ static int vfs_listen(int silent_addinuse_err)
304306
sizeof(g_sockaddr));
305307
if (ret < 0) {
306308
ret = -errno;
307-
if ((errno != EADDRINUSE) || !silent_addinuse_err) {
309+
if ((errno != EADDRINUSE) || !silent_addrinuse_err) {
308310
vfs_error("bind(%s) failed: %m", g_sockaddr.sun_path);
309311
}
310312
goto out_close;

‎src/ucs/sys/string.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,7 @@ size_t ucs_string_common_prefix_len(const char *str1, const char *str2)
353353
return (p1 - str1);
354354
}
355355

356-
static size_t
357-
ucs_path_common_parent_length(const char *path1, const char *path2)
356+
size_t ucs_path_common_parent_length(const char *path1, const char *path2)
358357
{
359358
size_t offset, parent_length;
360359

‎src/ucs/sys/string.h‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,18 @@ size_t ucs_string_count_char(const char *str, char c);
270270
size_t ucs_string_common_prefix_len(const char *str1, const char *str2);
271271

272272

273+
/*
274+
* Return the length of the common parent directory, without the trailing slash.
275+
* For example, for /sys/ab12 and /sys/ab23 return 4.
276+
*
277+
* @param path1 String pointing to first path.
278+
* @param path2 String pointing to second path.
279+
*
280+
* @return length of the common parent directory
281+
*/
282+
size_t ucs_path_common_parent_length(const char *path1, const char *path2);
283+
284+
273285
/*
274286
* Return the common parent directory, without the trailing slash.
275287
* 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);
279291
* @param common_path Buffer to hold the common parent path. The size of the
280292
* buffer must be large enough to hold the longest common
281293
* parent path, plus a '\0' terminator.
282-
* */
294+
*/
283295
void ucs_path_get_common_parent(const char *path1, const char *path2,
284296
char *common_path);
285297

‎src/ucs/vfs/base/vfs_obj.c‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ struct {
9191
{ \
9292
int khret; \
9393
khiter_t khiter = kh_put(_name, _h, _k, &khret); \
94-
ucs_assert((khret == UCS_KH_PUT_BUCKET_EMPTY) || \
95-
(khret == UCS_KH_PUT_BUCKET_CLEAR)); \
94+
ucs_assertv((khret == UCS_KH_PUT_BUCKET_EMPTY) || \
95+
(khret == UCS_KH_PUT_BUCKET_CLEAR), \
96+
"khret=%d path=%s", khret, (_node)->path); \
9697
kh_val(_h, khiter) = _node; \
9798
}
9899

@@ -169,31 +170,43 @@ static ucs_vfs_node_t *ucs_vfs_node_create(ucs_vfs_node_t *parent_node,
169170
const char *path,
170171
ucs_vfs_node_type_t type, void *obj)
171172
{
172-
ucs_vfs_node_t *node;
173+
ucs_vfs_node_t *node, *existing_node;
173174

174175
node = ucs_malloc(sizeof(*node) + strlen(path) + 1, "vfs_node");
175176
if (node == NULL) {
176-
ucs_error("Failed to allocate vfs_node");
177-
return NULL;
177+
ucs_error("failed to allocate vfs_node");
178+
goto err;
178179
}
179180

180-
/* initialize node */
181+
/* Initialize node */
181182
ucs_vfs_node_init(node, type, obj, parent_node);
182183
strcpy(node->path, path);
183184

184-
/* add to parent */
185-
ucs_list_add_head(&parent_node->children, &node->list);
186-
187-
/* add to obj hash */
185+
/* Add to object hash */
188186
if (node->obj != NULL) {
187+
existing_node = ucs_vfs_node_find_by_obj(node->obj);
188+
if (existing_node != NULL) {
189+
ucs_error("cannot add vfs node '%s', object %p (%s) already exists",
190+
node->path, node->obj, existing_node->path);
191+
goto err_free;
192+
}
193+
189194
ucs_vfs_kh_put(vfs_obj, &ucs_vfs_obj_context.obj_hash,
190195
(uintptr_t)node->obj, node);
191196
}
192197

193198
/* add to path hash */
194199
ucs_vfs_kh_put(vfs_path, &ucs_vfs_obj_context.path_hash, node->path, node);
195200

201+
/* add to parent */
202+
ucs_list_add_head(&parent_node->children, &node->list);
203+
196204
return node;
205+
206+
err_free:
207+
ucs_free(node);
208+
err:
209+
return NULL;
197210
}
198211

199212
/* must be called with lock held */
@@ -512,18 +525,22 @@ static void ucs_vfs_path_list_dir_cb(ucs_vfs_node_t *node,
512525
static void
513526
ucs_vfs_get_link_path(ucs_vfs_node_t *node, ucs_string_buffer_t *strb)
514527
{
515-
size_t i, n;
528+
size_t i, n, common_length;
516529

517530
ucs_assert(ucs_vfs_check_node(node, UCS_VFS_NODE_TYPE_SYM_LINK));
531+
ucs_assertv(node->target != NULL, "node=%p node->path=%s", node,
532+
node->path);
518533

519-
n = ucs_string_count_char(node->path, '/');
534+
common_length = ucs_path_common_parent_length(node->path,
535+
node->target->path);
536+
537+
n = ucs_string_count_char(&node->path[common_length], '/');
520538
for (i = 1; i < n; ++i) {
521539
ucs_string_buffer_appendf(strb, "../");
522540
}
523541

524-
if (node->target != NULL) {
525-
ucs_string_buffer_appendf(strb, "%s", &node->target->path[1]);
526-
}
542+
ucs_string_buffer_appendf(strb, "%s",
543+
&node->target->path[common_length + 1]);
527544
}
528545

529546
ucs_status_t
@@ -635,7 +652,6 @@ ucs_vfs_obj_add_sym_link(void *obj, void *target_obj, const char *rel_path, ...)
635652

636653
out:
637654
ucs_spin_unlock(&ucs_vfs_obj_context.lock);
638-
639655
return status;
640656
}
641657

‎test/gtest/ucs/test_vfs.cc‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,25 +285,35 @@ UCS_TEST_F(test_vfs_obj, check_ret) {
285285

286286
UCS_MT_TEST_F(test_vfs_obj, add_sym_link, 4)
287287
{
288-
static const char path_to_target[] = "target";
288+
static char parent_obj;
289+
ucs_vfs_obj_add_dir(NULL, &parent_obj, "parent");
289290

290-
static char target;
291-
ucs_vfs_obj_add_dir(NULL, &target, "target");
292-
ucs_vfs_obj_add_sym_link(NULL, &target, "link");
291+
// Add target directory and subdirectory
292+
// NOTE: Currently links to files are not supported
293+
static long target_obj = 0;
294+
ucs_vfs_obj_add_dir(&parent_obj, &target_obj, "target");
295+
296+
// Add source directory and link
297+
static char source_dir_obj;
298+
ucs_vfs_obj_add_dir(&parent_obj, &source_dir_obj, "source_dir");
299+
ucs_vfs_obj_add_sym_link(&source_dir_obj, &target_obj, "link");
300+
301+
const char *link_path = "/parent/source_dir/link";
302+
const char *path_to_target = "../target";
293303

294304
ucs_vfs_path_info_t path_info;
295-
EXPECT_UCS_OK(ucs_vfs_path_get_info("/link", &path_info));
305+
EXPECT_UCS_OK(ucs_vfs_path_get_info(link_path, &path_info));
296306
EXPECT_EQ(strlen(path_to_target), path_info.size);
297307
EXPECT_TRUE(path_info.mode & S_IFLNK);
298308

299309
ucs_string_buffer_t strb;
300310
ucs_string_buffer_init(&strb);
301-
EXPECT_UCS_OK(ucs_vfs_path_get_link("/link", &strb));
311+
EXPECT_UCS_OK(ucs_vfs_path_get_link(link_path, &strb));
302312
EXPECT_STREQ(path_to_target, ucs_string_buffer_cstr(&strb));
303313
ucs_string_buffer_cleanup(&strb);
304314

305315
barrier();
306-
ucs_vfs_obj_remove(&target);
316+
ucs_vfs_obj_remove(&parent_obj);
307317
}
308318

309319
UCS_TEST_F(test_vfs_obj, add_rw_check_ret) {

0 commit comments

Comments
 (0)