From 731bdd4d0c998da2f4d88b0902faaa9f5a1c805f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Fri, 6 Mar 2026 14:05:24 +0100 Subject: [PATCH 01/11] Start work on cmpdirs A utility to compare directories for equality. --- uspace/app/cmpdirs/cmpdirs.c | 467 +++++++++++++++++++++++++++++++++ uspace/app/cmpdirs/meson.build | 29 ++ uspace/app/meson.build | 1 + 3 files changed, 497 insertions(+) create mode 100644 uspace/app/cmpdirs/cmpdirs.c create mode 100644 uspace/app/cmpdirs/meson.build diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c new file mode 100644 index 0000000000..41b17e7706 --- /dev/null +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -0,0 +1,467 @@ +/* + * Copyright (c) 2026 Vít Skalický + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * - The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +/** @addtogroup cmpdirs + * @brief Recursively compare two directories for equality. Checks: directory and file tree, names, metadata and file content + * @{ + */ +/** + * @file + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define NAME "cmpdirs" +/** When reading the content of a file for comparison, how large should one chunk be. */ +#define CHUNK_SIZE 0x4000 +/** File names longer than this will be truncated. */ +#define NAME_BUFFER_SIZE 256 + +static void print_usage(void); + +/** Returns true if all directories and files in the filesystem tree of handle1 also exist in the filesystem tree of + * handle2 and in reverse and all their contents and metadata is equal. + * hande1 and handle2 are handles to a directory (or file). */ +static errno_t trees_equal(int handle1, int handle2, bool* equal_out); + +int main(int argc, char *argv[]) +{ + int optres, errflg = 0; + + /* Parse command-line options */ + while ((optres = getopt(argc, argv, "h")) != -1) { + switch (optres) { + case 'h': + print_usage(); + return 0; + + case '?': + fprintf(stderr, "Unrecognized option: -%c\n", optopt); + errflg++; + break; + + default: + fprintf(stderr, + "Unknown error while parsing command line options"); + errflg++; + break; + } + } + + // advance over already parsed options + argc -= optind; + argv = argv + optind; + + if (argc != 2) { + fprintf(stderr, "Wrong number of arguments. Two arguments are required.\n"); + errflg++; + } + + if (errflg) { + print_usage(); + return 1; + } + + errno_t rc = EOK; + + // initialize logging + rc = log_init(NAME); + if (rc != EOK) goto close_end; + + // lookup both directories + int handle1; + int handle2; + rc = vfs_lookup(argv[0], WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &handle1); + if (rc != EOK) goto close_end; + rc = vfs_lookup(argv[1], WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &handle2); + if (rc != EOK) goto close1; + + bool equal; + rc = trees_equal(handle1, handle2, &equal); + if (rc != EOK) goto close2; +close2: + vfs_put(handle2); +close1: + vfs_put(handle1); +close_end: + + if (rc != EOK) { + fprintf(stderr, "%d\n",rc); + fprintf(stderr, "An error occurred: %s: %s\n", str_error_name(rc), str_error(rc)); + return 2; + } + + return equal ? 0 : 1; +} + +struct entry { + int handle1; + int handle2; +}; + +/** Growable stack. */ +struct stack { + /** Allocated memory */ + struct entry *buffer; + /** How much is allocated */ + size_t capacity; + /** How much is used */ + size_t size; +}; + +size_t STACK_INIT_CAPACITY = 256; + +/** Create a new growable stack. Don't forget to free it in the end using stack_free */ +static errno_t stack_new(struct stack* s) { + s->size = 0; + s->capacity = STACK_INIT_CAPACITY; + s->buffer = malloc(sizeof(struct entry) * s->capacity); + if (s->buffer == NULL) { + return ENOMEM; + } + return EOK; +} + +/** Destroy a growable stack and free its memory */ +static void stack_free(struct stack* s) { + if (s->buffer != NULL) { + free(s->buffer); + } + s->buffer = NULL; + s->size = 0; + s->capacity = 0; +} + +/** Append an entry to the stack. Allocates larger memory if needed. */ +static errno_t stack_append(struct stack *s, struct entry e) { + if (s->capacity == 0 || s->buffer == NULL) { + // stack has already been freed before + return ENOENT; + } + if (s->size == s->capacity) { + // grow the stack + // allocate a new buffer twice the capacity of the old one and switch them. Also free the old one + struct entry *newbuffer = malloc(sizeof(struct entry) * s->capacity * 2); + if (newbuffer == NULL) { + return ENOMEM; + } + memcpy(newbuffer, s->buffer, sizeof(struct entry) * s->capacity); + free(s->buffer); + s->buffer = newbuffer; + s->capacity *= 2; + } + assert(s->size < s->capacity); + s->buffer[s->size] = e; + s->size++; + return EOK; +} + +/** Returns the last entry in the stack and removes it from it. Does not free any memory */ +static errno_t stack_pop(struct stack *s, struct entry *entry_out) { + if (s->size == 0) { + return EEMPTY; + } + s->size--; + (*entry_out) = s->buffer[s->size]; + return EOK; +} + +/** Writes the name of the entry at position pos in the directory represented by the handle into buffer out_name_buffer + * and writes the number of byte written to the buffer into out_name_len. The name should be null-terminated, unless it was too long. + * + * It assumes: + * - handle is open for reading (vfs_open()) + * + * There is no function for listing a directory in vfs.h and the functions from dirent.h kinda suck, so here I have my own. */ +static errno_t list_dir(int handle, aoff64_t pos, size_t buffer_size, char* out_name_buffer, ssize_t *out_name_len) { + errno_t rc = EOK; + rc = vfs_read_short(handle, pos, out_name_buffer, buffer_size, out_name_len); + return rc; +} + +/** Compares two vfs_stat_t structures in the sense that they represent the same data possibly across different filesystems. */ +static bool stat_equal(vfs_stat_t a, vfs_stat_t b) { + return a.is_file == b.is_file && + a.is_directory == b.is_directory && + a.size == b.size; +} + +/** Compares the contetn of 2 files. It assumes: + * - both handles represent a file, not dir + * - both files have the same size + * - both buffers are the size CHUNK_SIZE */ +static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int handle2, vfs_stat_t stat2, char* buffer2, bool *equal_out) { + errno_t rc = EOK; + bool equal = true; + aoff64_t pos = 0; + + //todo open the files? + + while (pos < stat1.size && equal) { + ssize_t read1, read2; + rc = vfs_read_short(handle1, pos, buffer1, CHUNK_SIZE, &read1); + if (rc != EOK) break; + rc = vfs_read_short(handle2, pos, buffer2, CHUNK_SIZE, &read2); + if (rc != EOK) break; + size_t read = min(read1, read2); + equal = (0 == memcmp(buffer1, buffer2, read)); + pos += read; + if (read1 == 0 && read2 == 0) { + log_msg(LOG_DEFAULT, LVL_ERROR, "Reading of files ended before their end. Considering them equal, but it is not supposed to happen."); + break; // this should not happen. Prevent infinite loop. + } + if (read == 0) { + // something bad has happened - one has read 0 while other not and we are not at the end yet. That is not possible + log_msg(LOG_DEFAULT, LVL_ERROR, "Reading of one file ended sooner than the other."); + equal = false; + break; + } + } + *equal_out = equal; + //todo close the files. do it here? + return rc; +} + +/** Checks if the directory/file tree of both handles is equal -- checks presence of files/dirs, content of all files, and metadata. */ +static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) { + // todo don't forget to check put/close all file handles + // Performs a depth-first search on handle1 and compares it to handle2. + errno_t rc = EOK; + bool equal = true; + // stack of handles to compare + // All handles on the stack are NOT open yet, but must be put() in the end + struct stack s; + rc = stack_new(&s); + if (rc != EOK) goto close_end; + + char *file_buffer1 = malloc(CHUNK_SIZE); + if (file_buffer1 == NULL) { + rc = ENOMEM; + goto close1; + } + char *file_buffer2 = malloc(CHUNK_SIZE); + if (file_buffer2 == NULL) { + rc = ENOMEM; + goto close2; + } + char name_buffer[NAME_BUFFER_SIZE]; + + // no need to add the initial entry into stack - it's the initial value in the for cycle + // rc = stack_append(&s, (struct entry){handle1, handle2}); + // if (rc != EOK) goto close3; + + // While there is something in the stack + for (struct entry item = {root_handle1, root_handle2}; // initial item + rc != EEMPTY; // was pop successful? + rc = stack_pop(&s, &item) + ) { + if (rc != EOK) { + // other error than EEMPTY + goto close3; + } + //TODO open the handles here + + // 1. compare metadata + // 2. open handles for reading + // 3. if files, compare contents + // 4. if directories, check that they contain the same items and add all items to stack + vfs_stat_t stat1; + rc = vfs_stat(item.handle1, &stat1); + if (rc != EOK) goto close_inner; + vfs_stat_t stat2; + rc = vfs_stat(item.handle2, &stat2); + if (rc != EOK) goto close_inner; + + rc = vfs_open(item.handle1, MODE_READ); + if (rc != EOK) goto close_inner; + rc = vfs_open(item.handle2, MODE_READ); + if (rc != EOK) goto close_inner; + + equal = equal && stat_equal(stat1, stat2); + if (!equal) { + goto close_inner; + } + if ((stat1.is_file && stat1.is_directory) || (!stat1.is_file && !stat1.is_directory)) { // sanity check + // WTF + log_msg(LOG_DEFAULT, LVL_FATAL, "Invalid entry encountered: It either claims to be both a directory and file at the same time or it claims to be neither."); + rc = EINVAL; + goto close_inner; + } + if (stat1.is_file) { + // check file content + bool equal2; + rc = content_equal(item.handle1, stat1, file_buffer1, item.handle2, stat2, file_buffer2, &equal2); + if (rc != EOK) goto close_inner; + equal = equal && equal2; + }else { + // check directory listing and add its items to the stack + + // 1. iterate over entries of handle1 -> gives you entry name + // 2. lookup it in handle1 + // 3. lookup it in handle2 + // 4. add both new handles to stack + // 5. iterate again over entries of handel2 -> gives you entry name + // 6. try to look it up in handle1 jut to check that it exists + + // 5. and 6. are probably redundant, since the sizes of both directories must be equal. + + for (ssize_t pos = 0; + ((size_t) pos) < stat1.size; + pos++) { + // 1. + ssize_t read; + rc = list_dir(item.handle1, pos, NAME_BUFFER_SIZE, name_buffer, &read); + log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); + if (rc != EOK) break; + // check that the string is null-terminated before the end of the buffer + if (str_nsize(name_buffer, NAME_BUFFER_SIZE) >= NAME_BUFFER_SIZE) { + rc = ENAMETOOLONG; + break; + } + // 2. + int entry_handle1; // don't forget to put + rc = vfs_walk(item.handle1, name_buffer, WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &entry_handle1); + if (rc != EOK) break; + // 3. + int entry_handle2; // must be put + rc= vfs_walk(item.handle2, name_buffer, WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &entry_handle2); + if (rc != EOK) { + if (rc == ENOENT) { // todo is ENOENT the correct one? + rc = EOK; + equal = false; + } + errno_t rc2 = vfs_put(entry_handle1); + (void) rc2; // if put failed, I guess there is nothing I can do about it + break; + } + + // 4. + struct entry newitem = {entry_handle1, entry_handle2}; + rc = stack_append(&s, newitem); + if (rc != EOK) { + errno_t rc2 = vfs_put(entry_handle1); + errno_t rc3 = vfs_put(entry_handle2); + (void) rc2; // if put failed, I guess there is nothing I can do about it + (void) rc3; + break; + } + } + if (rc != EOK) goto close_inner; + + // 5. + for (ssize_t pos = 0; + ((size_t) pos) < stat2.size; // must be equal to stat1.size anyway + pos++) { + // 5. + ssize_t read; + rc = list_dir(item.handle2, pos, NAME_BUFFER_SIZE, name_buffer, &read); + log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); + if (rc != EOK) break; + // check that the string is null-terminated before the end of the buffer + if (str_nsize(name_buffer, NAME_BUFFER_SIZE) >= NAME_BUFFER_SIZE) { + rc = ENAMETOOLONG; + break; + } + // 6. + int entry_handle; // don't forget to put + rc = vfs_walk(item.handle2, name_buffer, WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &entry_handle); + errno_t rc2 = vfs_put(entry_handle); + (void) rc2; // if put failed, I guess there is nothing I can do about it + if (rc != EOK) { + if (rc == ENOENT) { // todo is ENOENT the correct one? + rc = EOK; + equal = false; + } + break; + } + } + if (rc != EOK) goto close_inner; + } + + close_inner: + if (item.handle1 != root_handle1) { + errno_t rc2 = vfs_put(item.handle1); + (void) rc2; // if put failed, I guess there is nothing I can do about it + } + if (item.handle2 != root_handle2) { + errno_t rc2 = vfs_put(item.handle2); + (void) rc2; // if put failed, I guess there is nothing I can do about it + } + if (rc != EOK) goto close_start; + } + assert(rc == EEMPTY); + rc = EOK; +close_start: +close3: + free(file_buffer2); +close2: + free(file_buffer1); +close1: + (void) 0; // Clangd is not happy about declaration right after a label, so here is a dummy statement. + struct entry item = {}; + for (errno_t for_rc = stack_pop(&s, &item); + for_rc != EEMPTY; // was pop successful? + for_rc = stack_pop(&s, &item)){ + errno_t rc2 = vfs_put(item.handle1); + (void) rc2; // if put failed, I guess there is nothing I can do about it + rc2 = vfs_put(item.handle2); + (void) rc2; // if put failed, I guess there is nothing I can do about it + } + + stack_free(&s); +close_end: + if (rc == EOK) *equal_out = equal; + return rc; +} + + +static void print_usage(void) +{ + printf("Syntax: %s [] \n", NAME); + printf("Recursively compare two driectories for equality\n"); + printf("\n"); + printf("Options:\n"); + printf(" -h Print help\n"); + printf("Return code:\n"); + printf(" 0 and are equal\n"); + printf(" 1 and differ\n"); + printf(" 2 An error occurred. Details in stderr\n"); +} + +/** @} + */ diff --git a/uspace/app/cmpdirs/meson.build b/uspace/app/cmpdirs/meson.build new file mode 100644 index 0000000000..f955f938e0 --- /dev/null +++ b/uspace/app/cmpdirs/meson.build @@ -0,0 +1,29 @@ +# +# Copyright (c) 2013 Manuele Conti +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# - Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# - Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# - The name of the author may not be used to endorse or promote products +# derived from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR +# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES +# OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. +# IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +# NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF +# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# + +src = files('cmpdirs.c') diff --git a/uspace/app/meson.build b/uspace/app/meson.build index 4c58f9531a..70d3d6950d 100644 --- a/uspace/app/meson.build +++ b/uspace/app/meson.build @@ -34,6 +34,7 @@ apps = [ 'bithenge', 'blkdump', 'calculator', + 'cmpdirs', 'copy', 'corecfg', 'cpptest', From 79562579015624c4c99a2ace547e29f02d0d0f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Fri, 6 Mar 2026 15:33:21 +0100 Subject: [PATCH 02/11] Add logging to cmpdirs --- uspace/app/cmpdirs/cmpdirs.c | 88 +++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c index 41b17e7706..b0488a58b3 100644 --- a/uspace/app/cmpdirs/cmpdirs.c +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -52,6 +52,11 @@ /** File names longer than this will be truncated. */ #define NAME_BUFFER_SIZE 256 +// Macros to make handling error less clumbersome +#define _check_ok(rc, action, ...) if ((rc) != EOK) {fprintf(stderr, __VA_ARGS__); log_msg(LOG_DEFAULT, LVL_ERROR, __VA_ARGS__ ); action; } +#define check_ok(rc, label, ...) _check_ok(rc, goto label, __VA_ARGS__) +#define check_ok_break(rc, ...) _check_ok(rc, break, __VA_ARGS__) + static void print_usage(void); /** Returns true if all directories and files in the filesystem tree of handle1 also exist in the filesystem tree of @@ -77,7 +82,7 @@ int main(int argc, char *argv[]) default: fprintf(stderr, - "Unknown error while parsing command line options"); + "Unknown error while parsing command line options\n"); errflg++; break; } @@ -106,10 +111,10 @@ int main(int argc, char *argv[]) // lookup both directories int handle1; int handle2; - rc = vfs_lookup(argv[0], WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &handle1); - if (rc != EOK) goto close_end; - rc = vfs_lookup(argv[1], WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &handle2); - if (rc != EOK) goto close1; + rc = vfs_lookup(argv[0], 0, &handle1); + check_ok(rc, close_end, "Failed to lookup \"%s\"\n", argv[0]); + rc = vfs_lookup(argv[1], 0, &handle2); + check_ok(rc, close1, "Failed to lookup \"%s\"\n", argv[1]); bool equal; rc = trees_equal(handle1, handle2, &equal); @@ -224,21 +229,20 @@ static bool stat_equal(vfs_stat_t a, vfs_stat_t b) { /** Compares the contetn of 2 files. It assumes: * - both handles represent a file, not dir * - both files have the same size - * - both buffers are the size CHUNK_SIZE */ + * - both buffers are the size CHUNK_SIZE + * - both handle are open for reading */ static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int handle2, vfs_stat_t stat2, char* buffer2, bool *equal_out) { errno_t rc = EOK; bool equal = true; aoff64_t pos = 0; - //todo open the files? - while (pos < stat1.size && equal) { ssize_t read1, read2; rc = vfs_read_short(handle1, pos, buffer1, CHUNK_SIZE, &read1); - if (rc != EOK) break; + check_ok_break(rc, "Failed to read handle %d\n", handle1); rc = vfs_read_short(handle2, pos, buffer2, CHUNK_SIZE, &read2); - if (rc != EOK) break; - size_t read = min(read1, read2); + check_ok_break(rc, "Failed to read handle %d\n", handle2); + const size_t read = min(read1, read2); equal = (0 == memcmp(buffer1, buffer2, read)); pos += read; if (read1 == 0 && read2 == 0) { @@ -246,14 +250,13 @@ static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int h break; // this should not happen. Prevent infinite loop. } if (read == 0) { - // something bad has happened - one has read 0 while other not and we are not at the end yet. That is not possible + // something bad has happened - one has read 0 while other not and we are not at the end yet. That is not supposed to be possible log_msg(LOG_DEFAULT, LVL_ERROR, "Reading of one file ended sooner than the other."); equal = false; break; } } *equal_out = equal; - //todo close the files. do it here? return rc; } @@ -263,22 +266,23 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) // Performs a depth-first search on handle1 and compares it to handle2. errno_t rc = EOK; bool equal = true; + + log_msg(LOG_DEFAULT, LVL_DEBUG2, "Root handles are: %d, %d\n", root_handle1, root_handle2); + // stack of handles to compare // All handles on the stack are NOT open yet, but must be put() in the end struct stack s; rc = stack_new(&s); - if (rc != EOK) goto close_end; + check_ok(rc, close_end, "Failed to create stack\n"); char *file_buffer1 = malloc(CHUNK_SIZE); - if (file_buffer1 == NULL) { - rc = ENOMEM; - goto close1; - } + if (file_buffer1 == NULL) rc = ENOMEM; + check_ok(rc, close1, "No memory for file buffer #1 (%d bytes needed)\n", CHUNK_SIZE); + char *file_buffer2 = malloc(CHUNK_SIZE); - if (file_buffer2 == NULL) { - rc = ENOMEM; - goto close2; - } + if (file_buffer2 == NULL) rc = ENOMEM; + check_ok(rc, close2, "No memory for file buffer #2 (%d bytes needed)\n", CHUNK_SIZE); + char name_buffer[NAME_BUFFER_SIZE]; // no need to add the initial entry into stack - it's the initial value in the for cycle @@ -294,7 +298,6 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) // other error than EEMPTY goto close3; } - //TODO open the handles here // 1. compare metadata // 2. open handles for reading @@ -302,15 +305,15 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) // 4. if directories, check that they contain the same items and add all items to stack vfs_stat_t stat1; rc = vfs_stat(item.handle1, &stat1); - if (rc != EOK) goto close_inner; + check_ok(rc, close_inner, "Cannot stat handle %d\n", item.handle1); vfs_stat_t stat2; rc = vfs_stat(item.handle2, &stat2); - if (rc != EOK) goto close_inner; + check_ok(rc, close_inner, "Cannot stat handle %d\n", item.handle2); rc = vfs_open(item.handle1, MODE_READ); - if (rc != EOK) goto close_inner; + check_ok(rc, close_inner, "Cannot open handle %d\n", item.handle1); rc = vfs_open(item.handle2, MODE_READ); - if (rc != EOK) goto close_inner; + check_ok(rc, close_inner, "Cannot open handle %d\n", item.handle2); equal = equal && stat_equal(stat1, stat2); if (!equal) { @@ -318,9 +321,8 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) } if ((stat1.is_file && stat1.is_directory) || (!stat1.is_file && !stat1.is_directory)) { // sanity check // WTF - log_msg(LOG_DEFAULT, LVL_FATAL, "Invalid entry encountered: It either claims to be both a directory and file at the same time or it claims to be neither."); rc = EINVAL; - goto close_inner; + check_ok(rc, close_inner, "Invalid entry encountered: It either claims to be both a directory and file at the same time or it claims to be neither."); } if (stat1.is_file) { // check file content @@ -347,27 +349,28 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) ssize_t read; rc = list_dir(item.handle1, pos, NAME_BUFFER_SIZE, name_buffer, &read); log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); - if (rc != EOK) break; + check_ok_break(rc, "Failed to list handle %d\n", item.handle1); // check that the string is null-terminated before the end of the buffer if (str_nsize(name_buffer, NAME_BUFFER_SIZE) >= NAME_BUFFER_SIZE) { rc = ENAMETOOLONG; - break; } + check_ok_break(rc, "Name too long (limit is %d excluding null-terminator)", NAME_BUFFER_SIZE); // 2. int entry_handle1; // don't forget to put - rc = vfs_walk(item.handle1, name_buffer, WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &entry_handle1); - if (rc != EOK) break; + rc = vfs_walk(item.handle1, name_buffer, 0, &entry_handle1); + check_ok_break(rc, "(0) Failed to find entry \"%s\" in directory of handle %d\n", name_buffer, item.handle1); // 3. int entry_handle2; // must be put - rc= vfs_walk(item.handle2, name_buffer, WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &entry_handle2); + rc= vfs_walk(item.handle2, name_buffer, 0, &entry_handle2); if (rc != EOK) { + errno_t rc2 = vfs_put(entry_handle1); + (void) rc2; // if put failed, I guess there is nothing I can do about it if (rc == ENOENT) { // todo is ENOENT the correct one? rc = EOK; equal = false; + break; } - errno_t rc2 = vfs_put(entry_handle1); - (void) rc2; // if put failed, I guess there is nothing I can do about it - break; + check_ok_break(rc, "(1) Failed to find entry \"%s\" in directory of handle %d\n", name_buffer, item.handle2); } // 4. @@ -378,8 +381,8 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) errno_t rc3 = vfs_put(entry_handle2); (void) rc2; // if put failed, I guess there is nothing I can do about it (void) rc3; - break; } + check_ok_break(rc, "Failed to append to stack\n"); } if (rc != EOK) goto close_inner; @@ -391,23 +394,24 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) ssize_t read; rc = list_dir(item.handle2, pos, NAME_BUFFER_SIZE, name_buffer, &read); log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); - if (rc != EOK) break; + check_ok_break(rc, "Failed to list handle %d\n", item.handle2); // check that the string is null-terminated before the end of the buffer if (str_nsize(name_buffer, NAME_BUFFER_SIZE) >= NAME_BUFFER_SIZE) { rc = ENAMETOOLONG; - break; } + check_ok_break(rc, "Name too long (limit is %d excluding null-terminator)", NAME_BUFFER_SIZE); // 6. int entry_handle; // don't forget to put - rc = vfs_walk(item.handle2, name_buffer, WALK_DIRECTORY | WALK_REGULAR | WALK_MOUNT_POINT, &entry_handle); + rc = vfs_walk(item.handle1, name_buffer, 0, &entry_handle); errno_t rc2 = vfs_put(entry_handle); (void) rc2; // if put failed, I guess there is nothing I can do about it if (rc != EOK) { if (rc == ENOENT) { // todo is ENOENT the correct one? rc = EOK; equal = false; + break; } - break; + check_ok_break(rc, "(2) Failed to find entry \"%s\" in directory of handle %d\n", name_buffer, item.handle1); } } if (rc != EOK) goto close_inner; From 4a998bf9a9cab8e5b603eea4897e7ddc20bfe599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Thu, 12 Mar 2026 11:44:21 +0100 Subject: [PATCH 03/11] Fix directory listing The code for listing entries in a directory was wrong. --- uspace/app/cmpdirs/cmpdirs.c | 49 +++++++++--------------------------- 1 file changed, 12 insertions(+), 37 deletions(-) diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c index b0488a58b3..77ad36dbbd 100644 --- a/uspace/app/cmpdirs/cmpdirs.c +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -45,6 +45,7 @@ #include #include #include +#include #define NAME "cmpdirs" /** When reading the content of a file for comparison, how large should one chunk be. */ @@ -107,14 +108,17 @@ int main(int argc, char *argv[]) // initialize logging rc = log_init(NAME); if (rc != EOK) goto close_end; + logctl_set_log_level(NAME, LVL_DEBUG2); // lookup both directories int handle1; int handle2; rc = vfs_lookup(argv[0], 0, &handle1); check_ok(rc, close_end, "Failed to lookup \"%s\"\n", argv[0]); + log_msg(LOG_DEFAULT, LVL_DEBUG2, "fd of \"%s\" is %d", argv[0], handle1); rc = vfs_lookup(argv[1], 0, &handle2); check_ok(rc, close1, "Failed to lookup \"%s\"\n", argv[1]); + log_msg(LOG_DEFAULT, LVL_DEBUG2, "fd of \"%s\" is %d", argv[1], handle2); bool equal; rc = trees_equal(handle1, handle2, &equal); @@ -337,24 +341,25 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) // 2. lookup it in handle1 // 3. lookup it in handle2 // 4. add both new handles to stack - // 5. iterate again over entries of handel2 -> gives you entry name - // 6. try to look it up in handle1 jut to check that it exists - - // 5. and 6. are probably redundant, since the sizes of both directories must be equal. for (ssize_t pos = 0; ((size_t) pos) < stat1.size; - pos++) { + ) { // 1. ssize_t read; rc = list_dir(item.handle1, pos, NAME_BUFFER_SIZE, name_buffer, &read); log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); - check_ok_break(rc, "Failed to list handle %d\n", item.handle1); + check_ok_break(rc, "Failed (1) to list handle %d, pos %"PRIdn", total size of directory: %"PRIu64"\n", item.handle1, pos, stat1.size); + // Calculate the real length of the entry name (excluding null-terminator) + size_t name_size = str_nsize(name_buffer, NAME_BUFFER_SIZE); // check that the string is null-terminated before the end of the buffer - if (str_nsize(name_buffer, NAME_BUFFER_SIZE) >= NAME_BUFFER_SIZE) { + if (name_size >= NAME_BUFFER_SIZE) { rc = ENAMETOOLONG; } check_ok_break(rc, "Name too long (limit is %d excluding null-terminator)", NAME_BUFFER_SIZE); + // advance pos to after the name of the entry (add 1 for null-terminator) + pos += read; // NOLINT(*-narrowing-conversions) because name_size < NAME_BUFFER_SIZE + log_msg(LOG_DEFAULT, LVL_DEBUG2, "Listed %d: pos %"PRIdn", name: %s\n", item.handle1, pos, name_buffer); // 2. int entry_handle1; // don't forget to put rc = vfs_walk(item.handle1, name_buffer, 0, &entry_handle1); @@ -385,36 +390,6 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) check_ok_break(rc, "Failed to append to stack\n"); } if (rc != EOK) goto close_inner; - - // 5. - for (ssize_t pos = 0; - ((size_t) pos) < stat2.size; // must be equal to stat1.size anyway - pos++) { - // 5. - ssize_t read; - rc = list_dir(item.handle2, pos, NAME_BUFFER_SIZE, name_buffer, &read); - log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); - check_ok_break(rc, "Failed to list handle %d\n", item.handle2); - // check that the string is null-terminated before the end of the buffer - if (str_nsize(name_buffer, NAME_BUFFER_SIZE) >= NAME_BUFFER_SIZE) { - rc = ENAMETOOLONG; - } - check_ok_break(rc, "Name too long (limit is %d excluding null-terminator)", NAME_BUFFER_SIZE); - // 6. - int entry_handle; // don't forget to put - rc = vfs_walk(item.handle1, name_buffer, 0, &entry_handle); - errno_t rc2 = vfs_put(entry_handle); - (void) rc2; // if put failed, I guess there is nothing I can do about it - if (rc != EOK) { - if (rc == ENOENT) { // todo is ENOENT the correct one? - rc = EOK; - equal = false; - break; - } - check_ok_break(rc, "(2) Failed to find entry \"%s\" in directory of handle %d\n", name_buffer, item.handle1); - } - } - if (rc != EOK) goto close_inner; } close_inner: From 7c2a3c70963fbd19c92fc0ad00df390de519f44d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Mon, 16 Mar 2026 14:48:05 +0100 Subject: [PATCH 04/11] Improve comments --- uspace/app/cmpdirs/cmpdirs.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c index 77ad36dbbd..0029a35a20 100644 --- a/uspace/app/cmpdirs/cmpdirs.c +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -53,9 +53,11 @@ /** File names longer than this will be truncated. */ #define NAME_BUFFER_SIZE 256 -// Macros to make handling error less clumbersome +// Macros to make handling errors less cumbersome #define _check_ok(rc, action, ...) if ((rc) != EOK) {fprintf(stderr, __VA_ARGS__); log_msg(LOG_DEFAULT, LVL_ERROR, __VA_ARGS__ ); action; } +/** Print message (given by the variable arguments) to log and stderr if rc != EOK and then goto the specified label. */ #define check_ok(rc, label, ...) _check_ok(rc, goto label, __VA_ARGS__) +/** Print message (given by the variable arguments) to log and stderr if rc != EOK and then break. */ #define check_ok_break(rc, ...) _check_ok(rc, break, __VA_ARGS__) static void print_usage(void); @@ -138,6 +140,7 @@ int main(int argc, char *argv[]) return equal ? 0 : 1; } +/** Handles of 2 files or directories to compare */ struct entry { int handle1; int handle2; @@ -155,7 +158,7 @@ struct stack { size_t STACK_INIT_CAPACITY = 256; -/** Create a new growable stack. Don't forget to free it in the end using stack_free */ +/** Create a new growable stack. Don't forget to free it using stack_free when you are done using it. */ static errno_t stack_new(struct stack* s) { s->size = 0; s->capacity = STACK_INIT_CAPACITY; @@ -200,7 +203,7 @@ static errno_t stack_append(struct stack *s, struct entry e) { return EOK; } -/** Returns the last entry in the stack and removes it from it. Does not free any memory */ +/** Returns the last entry in the stack and removes it from it. Does not free any memory. */ static errno_t stack_pop(struct stack *s, struct entry *entry_out) { if (s->size == 0) { return EEMPTY; @@ -230,11 +233,11 @@ static bool stat_equal(vfs_stat_t a, vfs_stat_t b) { a.size == b.size; } -/** Compares the contetn of 2 files. It assumes: +/** Compares the content of 2 files. It assumes: * - both handles represent a file, not dir * - both files have the same size * - both buffers are the size CHUNK_SIZE - * - both handle are open for reading */ + * - both handles are open for reading */ static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int handle2, vfs_stat_t stat2, char* buffer2, bool *equal_out) { errno_t rc = EOK; bool equal = true; @@ -264,9 +267,8 @@ static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int h return rc; } -/** Checks if the directory/file tree of both handles is equal -- checks presence of files/dirs, content of all files, and metadata. */ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) { - // todo don't forget to check put/close all file handles + // todo Check once again that all file handles are put at the end // Performs a depth-first search on handle1 and compares it to handle2. errno_t rc = EOK; bool equal = true; @@ -289,10 +291,6 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) char name_buffer[NAME_BUFFER_SIZE]; - // no need to add the initial entry into stack - it's the initial value in the for cycle - // rc = stack_append(&s, (struct entry){handle1, handle2}); - // if (rc != EOK) goto close3; - // While there is something in the stack for (struct entry item = {root_handle1, root_handle2}; // initial item rc != EEMPTY; // was pop successful? From df4f7c30d6470268a01ea5ec78aa0cacf506662d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Mon, 16 Mar 2026 15:01:00 +0100 Subject: [PATCH 05/11] Add opendir_handle() ot dirent.c Add function to open a directory by its handle instead of pathname to dirent.c. --- uspace/lib/c/generic/dirent.c | 35 +++++++++++++++++++++++++---------- uspace/lib/c/include/dirent.h | 1 + 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/uspace/lib/c/generic/dirent.c b/uspace/lib/c/generic/dirent.c index 32101a2015..d830971bf8 100644 --- a/uspace/lib/c/generic/dirent.c +++ b/uspace/lib/c/generic/dirent.c @@ -46,13 +46,12 @@ struct __dirstream { aoff64_t pos; }; -/** Open directory. - * - * @param dirname Directory pathname +/** Open directory by its handle (a.k.a. file descriptor). * + * @param handle Directory handle * @return Non-NULL pointer on success. On error returns @c NULL and sets errno. */ -DIR *opendir(const char *dirname) +DIR *opendir_handle(int handle) { DIR *dirp = malloc(sizeof(DIR)); if (!dirp) { @@ -60,24 +59,40 @@ DIR *opendir(const char *dirname) return NULL; } + errno_t rc = vfs_open(handle, MODE_READ); + if (rc != EOK) { + free(dirp); + errno = rc; + return NULL; + } + + dirp->fd = handle; + dirp->pos = 0; + return dirp; +} + +/** Open directory by its pathname. + * + * @param dirname Directory pathname + * + * @return Non-NULL pointer on success. On error returns @c NULL and sets errno. + */ +DIR *opendir(const char *dirname) +{ int fd; errno_t rc = vfs_lookup(dirname, WALK_DIRECTORY, &fd); if (rc != EOK) { - free(dirp); errno = rc; return NULL; } - rc = vfs_open(fd, MODE_READ); + DIR *dirp = opendir_handle(fd); + rc = errno; if (rc != EOK) { - free(dirp); vfs_put(fd); errno = rc; return NULL; } - - dirp->fd = fd; - dirp->pos = 0; return dirp; } diff --git a/uspace/lib/c/include/dirent.h b/uspace/lib/c/include/dirent.h index 5be6eb8b9e..8c4f547cf4 100644 --- a/uspace/lib/c/include/dirent.h +++ b/uspace/lib/c/include/dirent.h @@ -45,6 +45,7 @@ struct dirent { typedef struct __dirstream DIR; +extern DIR *opendir_handle(int handle); extern DIR *opendir(const char *); extern struct dirent *readdir(DIR *); extern void rewinddir(DIR *); From 128a311e02aa7ad74377185bd929a82874cfcade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Mon, 16 Mar 2026 15:27:57 +0100 Subject: [PATCH 06/11] Use vfs_clone in opendir_handle() Before, the handle passed to it would be saved and later put by closedir. However, the handle is owned by the caller of opendir_handle() and it their responsibility to put it, not our. --- uspace/lib/c/generic/dirent.c | 37 +++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/uspace/lib/c/generic/dirent.c b/uspace/lib/c/generic/dirent.c index d830971bf8..c562b2e8eb 100644 --- a/uspace/lib/c/generic/dirent.c +++ b/uspace/lib/c/generic/dirent.c @@ -46,12 +46,8 @@ struct __dirstream { aoff64_t pos; }; -/** Open directory by its handle (a.k.a. file descriptor). - * - * @param handle Directory handle - * @return Non-NULL pointer on success. On error returns @c NULL and sets errno. - */ -DIR *opendir_handle(int handle) +/** Like opendir_handle, but takes ownership of the handle if successful. */ +static DIR *opendir_internal(int handle) { DIR *dirp = malloc(sizeof(DIR)); if (!dirp) { @@ -71,6 +67,31 @@ DIR *opendir_handle(int handle) return dirp; } +/** Open directory by its handle (a.k.a. file descriptor). + * + * @param handle Directory handle + * @return Non-NULL pointer on success. On error returns @c NULL and sets errno. + */ +DIR *opendir_handle(int handle) +{ + int my_handle; + errno_t rc = vfs_clone(handle, -1, false, &my_handle); // Clone the file handle, otherwise closedir would put the + // handle that was passed to us here by the caller and that we don't own. + if (rc != EOK) { + errno = rc; + return NULL; + } + + DIR *dirp = opendir_internal(my_handle); + rc = errno; + if (rc != EOK) { + vfs_put(my_handle); + errno = rc; + return NULL; + } + return dirp; +} + /** Open directory by its pathname. * * @param dirname Directory pathname @@ -86,7 +107,7 @@ DIR *opendir(const char *dirname) return NULL; } - DIR *dirp = opendir_handle(fd); + DIR *dirp = opendir_internal(fd); rc = errno; if (rc != EOK) { vfs_put(fd); @@ -96,7 +117,7 @@ DIR *opendir(const char *dirname) return dirp; } -/** Read directory entry. +/** Read current directory entry and advance to the next one. * * @param dirp Open directory * @return Non-NULL pointer to directory entry on success. On error returns From 8c35ebd2436462e92ef76474c72b0a2a5d964559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Mon, 16 Mar 2026 20:31:23 +0100 Subject: [PATCH 07/11] Modified cmpdirs to use the dirent.h --- uspace/app/cmpdirs/cmpdirs.c | 60 +++++++++++++++--------------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c index 0029a35a20..d6d987c77b 100644 --- a/uspace/app/cmpdirs/cmpdirs.c +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -34,6 +34,7 @@ * @file */ +#include #include #include #include @@ -50,8 +51,6 @@ #define NAME "cmpdirs" /** When reading the content of a file for comparison, how large should one chunk be. */ #define CHUNK_SIZE 0x4000 -/** File names longer than this will be truncated. */ -#define NAME_BUFFER_SIZE 256 // Macros to make handling errors less cumbersome #define _check_ok(rc, action, ...) if ((rc) != EOK) {fprintf(stderr, __VA_ARGS__); log_msg(LOG_DEFAULT, LVL_ERROR, __VA_ARGS__ ); action; } @@ -213,19 +212,6 @@ static errno_t stack_pop(struct stack *s, struct entry *entry_out) { return EOK; } -/** Writes the name of the entry at position pos in the directory represented by the handle into buffer out_name_buffer - * and writes the number of byte written to the buffer into out_name_len. The name should be null-terminated, unless it was too long. - * - * It assumes: - * - handle is open for reading (vfs_open()) - * - * There is no function for listing a directory in vfs.h and the functions from dirent.h kinda suck, so here I have my own. */ -static errno_t list_dir(int handle, aoff64_t pos, size_t buffer_size, char* out_name_buffer, ssize_t *out_name_len) { - errno_t rc = EOK; - rc = vfs_read_short(handle, pos, out_name_buffer, buffer_size, out_name_len); - return rc; -} - /** Compares two vfs_stat_t structures in the sense that they represent the same data possibly across different filesystems. */ static bool stat_equal(vfs_stat_t a, vfs_stat_t b) { return a.is_file == b.is_file && @@ -289,8 +275,6 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) if (file_buffer2 == NULL) rc = ENOMEM; check_ok(rc, close2, "No memory for file buffer #2 (%d bytes needed)\n", CHUNK_SIZE); - char name_buffer[NAME_BUFFER_SIZE]; - // While there is something in the stack for (struct entry item = {root_handle1, root_handle2}; // initial item rc != EEMPTY; // was pop successful? @@ -340,31 +324,33 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) // 3. lookup it in handle2 // 4. add both new handles to stack - for (ssize_t pos = 0; - ((size_t) pos) < stat1.size; - ) { - // 1. - ssize_t read; - rc = list_dir(item.handle1, pos, NAME_BUFFER_SIZE, name_buffer, &read); - log_msg(LOG_DEFAULT, LVL_DEBUG2, "Read directory entry name: result %s, read size: %"PRIdn".", str_error_name(rc), read); - check_ok_break(rc, "Failed (1) to list handle %d, pos %"PRIdn", total size of directory: %"PRIu64"\n", item.handle1, pos, stat1.size); + // Since the directories represented by handle1 and handle2 have equal size and all entries in handle1 are + // also present in handle2, then all entries in handle2 must also be present in handle1. Therefore they are equal. + + struct dirent *dp; + DIR *dirp = opendir_handle(item.handle1); + if (!dirp) { + rc = errno; + assert(rc != EOK); + } + check_ok(rc, close_inner, "Failed to open directory (handle %d", item.handle1); + + while ((dp = readdir(dirp))) { // 1. // Calculate the real length of the entry name (excluding null-terminator) - size_t name_size = str_nsize(name_buffer, NAME_BUFFER_SIZE); + size_t name_buffer_size = sizeof(dp->d_name); + size_t name_size = str_nsize(dp->d_name, name_buffer_size); // check that the string is null-terminated before the end of the buffer - if (name_size >= NAME_BUFFER_SIZE) { + if (name_size >= name_buffer_size) { rc = ENAMETOOLONG; } - check_ok_break(rc, "Name too long (limit is %d excluding null-terminator)", NAME_BUFFER_SIZE); - // advance pos to after the name of the entry (add 1 for null-terminator) - pos += read; // NOLINT(*-narrowing-conversions) because name_size < NAME_BUFFER_SIZE - log_msg(LOG_DEFAULT, LVL_DEBUG2, "Listed %d: pos %"PRIdn", name: %s\n", item.handle1, pos, name_buffer); + check_ok_break(rc, "Name too long (limit is %"PRIdn" excluding null-terminator)", name_buffer_size); // 2. int entry_handle1; // don't forget to put - rc = vfs_walk(item.handle1, name_buffer, 0, &entry_handle1); - check_ok_break(rc, "(0) Failed to find entry \"%s\" in directory of handle %d\n", name_buffer, item.handle1); + rc = vfs_walk(item.handle1, dp->d_name, 0, &entry_handle1); + check_ok_break(rc, "(0) Failed to find entry \"%s\" in directory of handle %d\n", dp->d_name, item.handle1); // 3. int entry_handle2; // must be put - rc= vfs_walk(item.handle2, name_buffer, 0, &entry_handle2); + rc= vfs_walk(item.handle2, dp->d_name, 0, &entry_handle2); if (rc != EOK) { errno_t rc2 = vfs_put(entry_handle1); (void) rc2; // if put failed, I guess there is nothing I can do about it @@ -373,7 +359,7 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) equal = false; break; } - check_ok_break(rc, "(1) Failed to find entry \"%s\" in directory of handle %d\n", name_buffer, item.handle2); + check_ok_break(rc, "(1) Failed to find entry \"%s\" in directory of handle %d\n", dp->d_name, item.handle2); } // 4. @@ -387,6 +373,7 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) } check_ok_break(rc, "Failed to append to stack\n"); } + closedir(dirp); if (rc != EOK) goto close_inner; } @@ -400,8 +387,9 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) (void) rc2; // if put failed, I guess there is nothing I can do about it } if (rc != EOK) goto close_start; + if (!equal) break; } - assert(rc == EEMPTY); + assert(rc == EEMPTY || !equal); rc = EOK; close_start: close3: From 7952b336d724f083cb8d6f8405136a1518af19f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Tue, 17 Mar 2026 13:11:14 +0100 Subject: [PATCH 08/11] Fix bad use of errno --- uspace/lib/c/generic/dirent.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/uspace/lib/c/generic/dirent.c b/uspace/lib/c/generic/dirent.c index c562b2e8eb..06e899402c 100644 --- a/uspace/lib/c/generic/dirent.c +++ b/uspace/lib/c/generic/dirent.c @@ -83,8 +83,8 @@ DIR *opendir_handle(int handle) } DIR *dirp = opendir_internal(my_handle); - rc = errno; - if (rc != EOK) { + if (!dirp) { + rc = errno; vfs_put(my_handle); errno = rc; return NULL; @@ -108,8 +108,8 @@ DIR *opendir(const char *dirname) } DIR *dirp = opendir_internal(fd); - rc = errno; - if (rc != EOK) { + if (!dirp) { + rc = errno; vfs_put(fd); errno = rc; return NULL; From b0178856327e4f27aa16082db4022897e58d8f86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Tue, 17 Mar 2026 13:16:27 +0100 Subject: [PATCH 09/11] Fix use of vfs_walk() Added slash to the beginning of file names in dirent so that they are an absolute path. vfs_walk() resolves path staring from the directory passed as parent, but the paths themselves must be absolute (start with /). --- uspace/app/cmpdirs/cmpdirs.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c index d6d987c77b..f96e467453 100644 --- a/uspace/app/cmpdirs/cmpdirs.c +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -333,7 +333,7 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) rc = errno; assert(rc != EOK); } - check_ok(rc, close_inner, "Failed to open directory (handle %d", item.handle1); + check_ok(rc, close_inner, "Failed to open directory of handle %d", item.handle1); while ((dp = readdir(dirp))) { // 1. // Calculate the real length of the entry name (excluding null-terminator) @@ -344,13 +344,18 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) rc = ENAMETOOLONG; } check_ok_break(rc, "Name too long (limit is %"PRIdn" excluding null-terminator)", name_buffer_size); + // add slash to the beginning of the name, because vfs_walk requires the path to be absolute (will be + // resolved relatively to the starting directory) + char name_with_slash[name_buffer_size+1]; + name_with_slash[0] = '/'; + str_cpy(name_with_slash + 1, name_size + 1, dp->d_name); // 2. int entry_handle1; // don't forget to put - rc = vfs_walk(item.handle1, dp->d_name, 0, &entry_handle1); - check_ok_break(rc, "(0) Failed to find entry \"%s\" in directory of handle %d\n", dp->d_name, item.handle1); + rc = vfs_walk(item.handle1, name_with_slash, 0, &entry_handle1); + check_ok_break(rc, "(0) Failed to find entry \"%s\" in directory of handle %d\n", name_with_slash, item.handle1); // 3. int entry_handle2; // must be put - rc= vfs_walk(item.handle2, dp->d_name, 0, &entry_handle2); + rc= vfs_walk(item.handle2, name_with_slash, 0, &entry_handle2); if (rc != EOK) { errno_t rc2 = vfs_put(entry_handle1); (void) rc2; // if put failed, I guess there is nothing I can do about it @@ -359,7 +364,7 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) equal = false; break; } - check_ok_break(rc, "(1) Failed to find entry \"%s\" in directory of handle %d\n", dp->d_name, item.handle2); + check_ok_break(rc, "(1) Failed to find entry \"%s\" in directory of handle %d\n", name_with_slash, item.handle2); } // 4. From 74c69dc7293d72b2a846256f01afa47e0f149c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Tue, 17 Mar 2026 13:43:08 +0100 Subject: [PATCH 10/11] Fix code style --- uspace/app/cmpdirs/cmpdirs.c | 94 ++++++++++++++++++++--------------- uspace/lib/c/generic/dirent.c | 5 +- 2 files changed, 58 insertions(+), 41 deletions(-) diff --git a/uspace/app/cmpdirs/cmpdirs.c b/uspace/app/cmpdirs/cmpdirs.c index f96e467453..315ef38b08 100644 --- a/uspace/app/cmpdirs/cmpdirs.c +++ b/uspace/app/cmpdirs/cmpdirs.c @@ -63,8 +63,9 @@ static void print_usage(void); /** Returns true if all directories and files in the filesystem tree of handle1 also exist in the filesystem tree of * handle2 and in reverse and all their contents and metadata is equal. - * hande1 and handle2 are handles to a directory (or file). */ -static errno_t trees_equal(int handle1, int handle2, bool* equal_out); + * hande1 and handle2 are handles to a directory (or file). + */ +static errno_t trees_equal(int handle1, int handle2, bool *equal_out); int main(int argc, char *argv[]) { @@ -108,7 +109,8 @@ int main(int argc, char *argv[]) // initialize logging rc = log_init(NAME); - if (rc != EOK) goto close_end; + if (rc != EOK) + goto close_end; logctl_set_log_level(NAME, LVL_DEBUG2); // lookup both directories @@ -123,7 +125,8 @@ int main(int argc, char *argv[]) bool equal; rc = trees_equal(handle1, handle2, &equal); - if (rc != EOK) goto close2; + if (rc != EOK) + goto close2; close2: vfs_put(handle2); close1: @@ -131,7 +134,7 @@ int main(int argc, char *argv[]) close_end: if (rc != EOK) { - fprintf(stderr, "%d\n",rc); + fprintf(stderr, "%d\n", rc); fprintf(stderr, "An error occurred: %s: %s\n", str_error_name(rc), str_error(rc)); return 2; } @@ -158,7 +161,8 @@ struct stack { size_t STACK_INIT_CAPACITY = 256; /** Create a new growable stack. Don't forget to free it using stack_free when you are done using it. */ -static errno_t stack_new(struct stack* s) { +static errno_t stack_new(struct stack *s) +{ s->size = 0; s->capacity = STACK_INIT_CAPACITY; s->buffer = malloc(sizeof(struct entry) * s->capacity); @@ -169,7 +173,8 @@ static errno_t stack_new(struct stack* s) { } /** Destroy a growable stack and free its memory */ -static void stack_free(struct stack* s) { +static void stack_free(struct stack *s) +{ if (s->buffer != NULL) { free(s->buffer); } @@ -179,7 +184,8 @@ static void stack_free(struct stack* s) { } /** Append an entry to the stack. Allocates larger memory if needed. */ -static errno_t stack_append(struct stack *s, struct entry e) { +static errno_t stack_append(struct stack *s, struct entry e) +{ if (s->capacity == 0 || s->buffer == NULL) { // stack has already been freed before return ENOENT; @@ -203,7 +209,8 @@ static errno_t stack_append(struct stack *s, struct entry e) { } /** Returns the last entry in the stack and removes it from it. Does not free any memory. */ -static errno_t stack_pop(struct stack *s, struct entry *entry_out) { +static errno_t stack_pop(struct stack *s, struct entry *entry_out) +{ if (s->size == 0) { return EEMPTY; } @@ -213,18 +220,21 @@ static errno_t stack_pop(struct stack *s, struct entry *entry_out) { } /** Compares two vfs_stat_t structures in the sense that they represent the same data possibly across different filesystems. */ -static bool stat_equal(vfs_stat_t a, vfs_stat_t b) { +static bool stat_equal(vfs_stat_t a, vfs_stat_t b) +{ return a.is_file == b.is_file && - a.is_directory == b.is_directory && - a.size == b.size; + a.is_directory == b.is_directory && + a.size == b.size; } /** Compares the content of 2 files. It assumes: * - both handles represent a file, not dir * - both files have the same size * - both buffers are the size CHUNK_SIZE - * - both handles are open for reading */ -static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int handle2, vfs_stat_t stat2, char* buffer2, bool *equal_out) { + * - both handles are open for reading + */ +static errno_t content_equal(int handle1, vfs_stat_t stat1, char *buffer1, int handle2, vfs_stat_t stat2, char *buffer2, bool *equal_out) +{ errno_t rc = EOK; bool equal = true; aoff64_t pos = 0; @@ -253,7 +263,8 @@ static errno_t content_equal(int handle1, vfs_stat_t stat1, char* buffer1, int h return rc; } -static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) { +static errno_t trees_equal(int root_handle1, int root_handle2, bool *equal_out) +{ // todo Check once again that all file handles are put at the end // Performs a depth-first search on handle1 and compares it to handle2. errno_t rc = EOK; @@ -268,18 +279,19 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) check_ok(rc, close_end, "Failed to create stack\n"); char *file_buffer1 = malloc(CHUNK_SIZE); - if (file_buffer1 == NULL) rc = ENOMEM; + if (file_buffer1 == NULL) + rc = ENOMEM; check_ok(rc, close1, "No memory for file buffer #1 (%d bytes needed)\n", CHUNK_SIZE); char *file_buffer2 = malloc(CHUNK_SIZE); - if (file_buffer2 == NULL) rc = ENOMEM; + if (file_buffer2 == NULL) + rc = ENOMEM; check_ok(rc, close2, "No memory for file buffer #2 (%d bytes needed)\n", CHUNK_SIZE); // While there is something in the stack - for (struct entry item = {root_handle1, root_handle2}; // initial item - rc != EEMPTY; // was pop successful? - rc = stack_pop(&s, &item) - ) { + for (struct entry item = { root_handle1, root_handle2 }; // initial item + rc != EEMPTY; // was pop successful? + rc = stack_pop(&s, &item)) { if (rc != EOK) { // other error than EEMPTY goto close3; @@ -314,9 +326,10 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) // check file content bool equal2; rc = content_equal(item.handle1, stat1, file_buffer1, item.handle2, stat2, file_buffer2, &equal2); - if (rc != EOK) goto close_inner; + if (rc != EOK) + goto close_inner; equal = equal && equal2; - }else { + } else { // check directory listing and add its items to the stack // 1. iterate over entries of handle1 -> gives you entry name @@ -343,10 +356,10 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) if (name_size >= name_buffer_size) { rc = ENAMETOOLONG; } - check_ok_break(rc, "Name too long (limit is %"PRIdn" excluding null-terminator)", name_buffer_size); + check_ok_break(rc, "Name too long (limit is %" PRIdn " excluding null-terminator)", name_buffer_size); // add slash to the beginning of the name, because vfs_walk requires the path to be absolute (will be // resolved relatively to the starting directory) - char name_with_slash[name_buffer_size+1]; + char name_with_slash[name_buffer_size + 1]; name_with_slash[0] = '/'; str_cpy(name_with_slash + 1, name_size + 1, dp->d_name); // 2. @@ -355,7 +368,7 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) check_ok_break(rc, "(0) Failed to find entry \"%s\" in directory of handle %d\n", name_with_slash, item.handle1); // 3. int entry_handle2; // must be put - rc= vfs_walk(item.handle2, name_with_slash, 0, &entry_handle2); + rc = vfs_walk(item.handle2, name_with_slash, 0, &entry_handle2); if (rc != EOK) { errno_t rc2 = vfs_put(entry_handle1); (void) rc2; // if put failed, I guess there is nothing I can do about it @@ -368,7 +381,7 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) } // 4. - struct entry newitem = {entry_handle1, entry_handle2}; + struct entry newitem = { entry_handle1, entry_handle2 }; rc = stack_append(&s, newitem); if (rc != EOK) { errno_t rc2 = vfs_put(entry_handle1); @@ -379,7 +392,8 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) check_ok_break(rc, "Failed to append to stack\n"); } closedir(dirp); - if (rc != EOK) goto close_inner; + if (rc != EOK) + goto close_inner; } close_inner: @@ -391,8 +405,10 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) errno_t rc2 = vfs_put(item.handle2); (void) rc2; // if put failed, I guess there is nothing I can do about it } - if (rc != EOK) goto close_start; - if (!equal) break; + if (rc != EOK) + goto close_start; + if (!equal) + break; } assert(rc == EEMPTY || !equal); rc = EOK; @@ -403,23 +419,23 @@ static errno_t trees_equal(int root_handle1, int root_handle2, bool* equal_out) free(file_buffer1); close1: (void) 0; // Clangd is not happy about declaration right after a label, so here is a dummy statement. - struct entry item = {}; + struct entry item = { }; for (errno_t for_rc = stack_pop(&s, &item); - for_rc != EEMPTY; // was pop successful? - for_rc = stack_pop(&s, &item)){ - errno_t rc2 = vfs_put(item.handle1); - (void) rc2; // if put failed, I guess there is nothing I can do about it - rc2 = vfs_put(item.handle2); - (void) rc2; // if put failed, I guess there is nothing I can do about it + for_rc != EEMPTY; // was pop successful? + for_rc = stack_pop(&s, &item)) { + errno_t rc2 = vfs_put(item.handle1); + (void) rc2; // if put failed, I guess there is nothing I can do about it + rc2 = vfs_put(item.handle2); + (void) rc2; // if put failed, I guess there is nothing I can do about it } stack_free(&s); close_end: - if (rc == EOK) *equal_out = equal; + if (rc == EOK) + *equal_out = equal; return rc; } - static void print_usage(void) { printf("Syntax: %s [] \n", NAME); diff --git a/uspace/lib/c/generic/dirent.c b/uspace/lib/c/generic/dirent.c index 06e899402c..2daa8de284 100644 --- a/uspace/lib/c/generic/dirent.c +++ b/uspace/lib/c/generic/dirent.c @@ -75,8 +75,9 @@ static DIR *opendir_internal(int handle) DIR *opendir_handle(int handle) { int my_handle; - errno_t rc = vfs_clone(handle, -1, false, &my_handle); // Clone the file handle, otherwise closedir would put the - // handle that was passed to us here by the caller and that we don't own. + // Clone the file handle, otherwise closedir would put the + // handle that was passed to us here by the caller and that we don't own. + errno_t rc = vfs_clone(handle, -1, false, &my_handle); if (rc != EOK) { errno = rc; return NULL; From 34832c3e4df70614df5caa7d44b985f0460ef8ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADt=20Skalick=C3=BD?= Date: Wed, 18 Mar 2026 13:46:13 +0100 Subject: [PATCH 11/11] Update license text and docs for vfs_walk and vfs_lookup_internal --- uspace/lib/c/generic/dirent.c | 1 + uspace/lib/c/generic/vfs/vfs.c | 6 +++++- uspace/lib/c/include/dirent.h | 1 + uspace/srv/vfs/vfs_lookup.c | 7 +++++-- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/uspace/lib/c/generic/dirent.c b/uspace/lib/c/generic/dirent.c index 2daa8de284..cd811162fd 100644 --- a/uspace/lib/c/generic/dirent.c +++ b/uspace/lib/c/generic/dirent.c @@ -1,5 +1,6 @@ /* * Copyright (c) 2008 Jakub Jermar + * Copyright (c) 2026 Vít Skalický * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/uspace/lib/c/generic/vfs/vfs.c b/uspace/lib/c/generic/vfs/vfs.c index 99e4e262d9..21d30532fd 100644 --- a/uspace/lib/c/generic/vfs/vfs.c +++ b/uspace/lib/c/generic/vfs/vfs.c @@ -1258,7 +1258,11 @@ errno_t vfs_unmount_path(const char *mpp) /** Walk a path starting in a parent node * * @param parent File handle of the parent node where the walk starts - * @param path Parent-relative path to be walked + * @param path Parent-relative absolute path to be walked (an + * absolute path with the parent being the root). + * If /foo is parent and path is "/bar/baz", the + * result would be /foo/bar/baz. Just "bar/baz" is + * not valid. * @param flags Flags influencing the walk * @param[out] handle File handle representing the result on success. * diff --git a/uspace/lib/c/include/dirent.h b/uspace/lib/c/include/dirent.h index 8c4f547cf4..7ab50680e9 100644 --- a/uspace/lib/c/include/dirent.h +++ b/uspace/lib/c/include/dirent.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2008 Jakub Jermar + * Copyright (c) 2026 Vít Skalický * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/uspace/srv/vfs/vfs_lookup.c b/uspace/srv/vfs/vfs_lookup.c index 1932513fc0..0093a9a323 100644 --- a/uspace/srv/vfs/vfs_lookup.c +++ b/uspace/srv/vfs/vfs_lookup.c @@ -324,8 +324,11 @@ static errno_t _vfs_lookup_internal(vfs_node_t *base, char *path, int lflag, /** Perform a path lookup. * * @param base The file from which to perform the lookup. - * @param path Path to be resolved; it must be a NULL-terminated - * string. + * @param path Path to be resolved; It must be an absolute path + * which get resolved relatively to base; it must be a + * NULL-terminated string. Example: If /foo is parent + * and path is "/bar/baz", the result would be /foo/bar/baz. + * Just "bar/baz" is not valid. * @param lflag Flags to be used during lookup. * @param result Empty structure where the lookup result will be stored. * Can be NULL.