From e405d0109e08096fbff206982caf5ecc042d8a9e Mon Sep 17 00:00:00 2001 From: opencode Date: Sun, 11 Oct 2026 18:31:13 +0000 Subject: [PATCH] fix: deduplicate non-consecutive directory entries in readdir Use a dmlist_context_t in directory_node_t to track already-emitted directory names across the entire readdir traversal, not just consecutive entries. Previously file entries between same-parent drivers would break the dedup loop and produce duplicate directory entries. --- src/dmdevfs.c | 107 ++++++++++++------------ tests/fixtures/config/readdir_dedup.ini | 19 +++++ 2 files changed, 73 insertions(+), 53 deletions(-) create mode 100644 tests/fixtures/config/readdir_dedup.ini diff --git a/src/dmdevfs.c b/src/dmdevfs.c index a843f48..9b0eb12 100644 --- a/src/dmdevfs.c +++ b/src/dmdevfs.c @@ -145,8 +145,9 @@ typedef struct typedef struct { - driver_node_t* driver; // Last driver - char* directory_path; // Directory path + driver_node_t* driver; // Last driver + char* directory_path; // Directory path + dmlist_context_t* seen_dirs; // Already-emitted directory names (dedup) } directory_node_t; /** @@ -1117,10 +1118,17 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, int, _opendir, (dmfsi_context_t ct } dir_node->driver = get_next_driver_node(ctx, NULL, path); dir_node->directory_path = Dmod_StrDup(path); - - *dp = dir_node; - + dir_node->seen_dirs = dmlist_create(); + if (dir_node->directory_path == NULL || dir_node->seen_dirs == NULL) + { + DMOD_LOG_ERROR("Failed to allocate directory node resources\n"); + Dmod_Free(dir_node->directory_path); + dmlist_destroy(dir_node->seen_dirs); + Dmod_Free(dir_node); + return DMFSI_ERR_GENERAL; + } + *dp = dir_node; return DMFSI_OK; } @@ -1140,74 +1148,61 @@ dmod_dmfsi_dif_api_declaration( 2.0, dmdevfs, int, _readdir, (dmfsi_context_t ct { return DMFSI_ERR_NOT_FOUND; // No more entries } - driver_node_t* driver = dir_node->driver; - path_t parent_dir; - if (read_driver_parent_directory(dir_node->driver, parent_dir, sizeof(parent_dir)) != 0) - { - DMOD_LOG_ERROR("Failed to read parent directory for driver\n"); - return DMFSI_ERR_GENERAL; - } + for (;;) { + driver_node_t* driver = dir_node->driver; - bool file_should_be_listed = compare_paths_ignore_trailing_slash(dir_node->directory_path, parent_dir) == 0; - if(file_should_be_listed) - { - // Extract basename from the full path for the directory entry - read_base_name(driver->path, entry->name, sizeof(entry->name)); - - dmdrvi_stat_t stat = {0}; - int res = driver_stat(driver, driver->path, &stat); - if (res != 0) + path_t parent_dir; + if (read_driver_parent_directory(driver, parent_dir, sizeof(parent_dir)) != 0) { - DMOD_LOG_ERROR("Failed to get file stats for: %s\n", driver->path); + DMOD_LOG_ERROR("Failed to read parent directory for driver\n"); return DMFSI_ERR_GENERAL; } - entry->size = stat.size; - entry->attr = stat.mode & ~DMFSI_ATTR_DIRECTORY; - } - else - { - // Extract the immediate subdirectory name relative to the listing directory. - // E.g. listing "/" with a driver whose parent is "dmgpio8/" yields "dmgpio8". - read_next_subdir_name(dir_node->directory_path, parent_dir, entry->name, sizeof(entry->name)); - entry->size = 0; - entry->attr = DMFSI_ATTR_DIRECTORY; - } + bool file_should_be_listed = compare_paths_ignore_trailing_slash(dir_node->directory_path, parent_dir) == 0; - // Move to next driver for subsequent call. - // When this entry is a directory, skip subsequent drivers that would emit - // the same immediate subdirectory name to avoid duplicate directory entries. - driver_node_t* next_driver = get_next_driver_node(ctx, driver, dir_node->directory_path); - if (!file_should_be_listed) - { - while (next_driver != NULL) + if (file_should_be_listed) { - path_t next_parent_dir; - if (read_driver_parent_directory(next_driver, next_parent_dir, sizeof(next_parent_dir)) != 0) + read_base_name(driver->path, entry->name, sizeof(entry->name)); + + dmdrvi_stat_t stat = {0}; + int res = driver_stat(driver, driver->path, &stat); + if (res != 0) { - DMOD_LOG_ERROR("Failed to read parent directory for next driver\n"); + DMOD_LOG_ERROR("Failed to get file stats for: %s\n", driver->path); return DMFSI_ERR_GENERAL; } - bool next_is_file_in_listed_dir = compare_paths_ignore_trailing_slash(dir_node->directory_path, next_parent_dir) == 0; - if (next_is_file_in_listed_dir) + entry->size = stat.size; + entry->attr = stat.mode & ~DMFSI_ATTR_DIRECTORY; + } + else + { + path_t subdir_name; + read_next_subdir_name(dir_node->directory_path, parent_dir, subdir_name, sizeof(subdir_name)); + + // Skip directories that were already emitted (non-consecutive dedup) + if (dmlist_find(dir_node->seen_dirs, subdir_name, (dmlist_compare_func_t)strcmp) != NULL) { - break; + dir_node->driver = get_next_driver_node(ctx, driver, dir_node->directory_path); + continue; } - path_t next_subdir_name; - read_next_subdir_name(dir_node->directory_path, next_parent_dir, next_subdir_name, sizeof(next_subdir_name)); - if (strcmp(next_subdir_name, entry->name) != 0) + char* dup = Dmod_StrDup(subdir_name); + if (dup != NULL) { - break; + dmlist_push_back(dir_node->seen_dirs, dup); } - next_driver = get_next_driver_node(ctx, next_driver, dir_node->directory_path); + strncpy(entry->name, subdir_name, sizeof(entry->name)); + entry->name[sizeof(entry->name) - 1] = '\0'; + entry->size = 0; + entry->attr = DMFSI_ATTR_DIRECTORY; } + + dir_node->driver = get_next_driver_node(ctx, driver, dir_node->directory_path); + return DMFSI_OK; } - dir_node->driver = next_driver; - return DMFSI_OK; } /** @@ -1223,6 +1218,12 @@ dmod_dmfsi_dif_api_declaration( 1.0, dmdevfs, int, _closedir, (dmfsi_context_t c directory_node_t* dir_node = (directory_node_t*)dp; Dmod_Free(dir_node->directory_path); + size_t count = dmlist_size(dir_node->seen_dirs); + for (size_t i = 0; i < count; i++) + { + Dmod_Free(dmlist_get(dir_node->seen_dirs, i)); + } + dmlist_destroy(dir_node->seen_dirs); Dmod_Free(dir_node); return DMFSI_OK; } diff --git a/tests/fixtures/config/readdir_dedup.ini b/tests/fixtures/config/readdir_dedup.ini new file mode 100644 index 0000000..37a104f --- /dev/null +++ b/tests/fixtures/config/readdir_dedup.ini @@ -0,0 +1,19 @@ +; Config that produces non-consecutive duplicate directories in readdir. +; mockA and mockC share the same parent /dmdevfs_mockdrv0/ (same major=0), +; but mockB sits between them (no major -> parent="/"). +; This exercises the non-consecutive directory dedup logic. +; +; Expected listing of "/": null, dmdevfs_mockdrv0, dmdevfs_mockdrv +[mockA] +driver_name=dmdevfs_mockdrv +driver_order=10 +major=0 + +[mockB] +driver_name=dmdevfs_mockdrv +driver_order=20 + +[mockC] +driver_name=dmdevfs_mockdrv +driver_order=30 +major=0