From ab46cb0bbdf52c977d43df985727184fcb0d2926 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 8 Jun 2024 13:55:29 -0500 Subject: [PATCH] Reverted some root-related errors to EXIST/ISDIR Changed: - lfsr_mkdir(&lfs, "/") => LFS_ERR_EXIST - lfsr_file_open(&lfs, &file, "/", *) => LFS_ERR_ISDIR Unchanged: - lfsr_remove(&lfs, "/") => LFS_ERR_INVAL - lfsr_rename(&lfs, "/", *) => LFS_ERR_INVAL - lfsr_rename(&lfs, *, "/") => LFS_ERR_INVAL This better matches what Linux, etc, does: prefering a normal dir-related error unless the only issue is that the dir in question is the root. Though Linux, etc, usually return EBUSY, which seems to also be used for special device files. We could add LFS_ERR_BUSY, but I'm not sure it's really worth it for such a rare error. It's not like the name would help anything... Internally, lfsr_mtree_pathlookup always returns LFS_ERR_INVAL for root, so this unfortunately requires a bit more code to map to the correct errors: code stack before: 33598 2592 after 33634 (+0.1%) 2592 (+0.0%) --- lfs.c | 6 +++--- tests/test_dirs.toml | 4 ++-- tests/test_files.toml | 10 +++++----- tests/test_paths.toml | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lfs.c b/lfs.c index 893d9ab4..b295d0b4 100644 --- a/lfs.c +++ b/lfs.c @@ -9116,11 +9116,11 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) { err = lfsr_mtree_pathlookup(lfs, &lfs->mtree, path, &mdir, &tag, &did, &name, &name_size); - if (err && err != LFS_ERR_EXIST) { + if (err && err != LFS_ERR_EXIST && err != LFS_ERR_INVAL) { return err; } // already exists? note orphans don't really exist - bool exists = (err == LFS_ERR_EXIST); + bool exists = (err == LFS_ERR_EXIST || err == LFS_ERR_INVAL); if (exists && tag != LFSR_TAG_ORPHAN) { return LFS_ERR_EXIST; } @@ -10005,7 +10005,7 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file, int err = lfsr_mtree_pathlookup(lfs, &lfs->mtree, path, &file->o.mdir, &tag, &did, &name, &name_size); - if (err && err != LFS_ERR_EXIST) { + if (err && err != LFS_ERR_EXIST && err != LFS_ERR_INVAL) { return err; } diff --git a/tests/test_dirs.toml b/tests/test_dirs.toml index 44d2d256..84409c02 100644 --- a/tests/test_dirs.toml +++ b/tests/test_dirs.toml @@ -283,14 +283,14 @@ code = ''' } // try to make root, which doesn't make sense - lfsr_mkdir(&lfs, "/") => LFS_ERR_INVAL; + lfsr_mkdir(&lfs, "/") => LFS_ERR_EXIST; // make a directory err = lfsr_mkdir(&lfs, "ardvark"); assert(!err || (TEST_PLS && err == LFS_ERR_EXIST)); // try to make root, which doesn't make sense - lfsr_mkdir(&lfs, "/") => LFS_ERR_INVAL; + lfsr_mkdir(&lfs, "/") => LFS_ERR_EXIST; for (int remount = 0; remount < 2; remount++) { // remount? diff --git a/tests/test_files.toml b/tests/test_files.toml index ca584678..b426e5c0 100644 --- a/tests/test_files.toml +++ b/tests/test_files.toml @@ -456,16 +456,16 @@ code = ''' // try reading our root as a file lfsr_file_t file; - lfsr_file_open(&lfs, &file, "/", LFS_O_RDONLY) => LFS_ERR_INVAL; + lfsr_file_open(&lfs, &file, "/", LFS_O_RDONLY) => LFS_ERR_ISDIR; // try writing our root as a file - lfsr_file_open(&lfs, &file, "/", LFS_O_WRONLY) => LFS_ERR_INVAL; + lfsr_file_open(&lfs, &file, "/", LFS_O_WRONLY) => LFS_ERR_ISDIR; lfsr_file_open(&lfs, &file, "/", - LFS_O_WRONLY | LFS_O_TRUNC) => LFS_ERR_INVAL; + LFS_O_WRONLY | LFS_O_TRUNC) => LFS_ERR_ISDIR; lfsr_file_open(&lfs, &file, "/", - LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_INVAL; + LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR; lfsr_file_open(&lfs, &file, "/", - LFS_O_WRONLY | LFS_O_CREAT | LFS_O_TRUNC) => LFS_ERR_INVAL; + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_TRUNC) => LFS_ERR_ISDIR; // try rename a file on top of our directory lfsr_file_open(&lfs, &file, "not_hello", diff --git a/tests/test_paths.toml b/tests/test_paths.toml index ab801b67..4d4046d3 100644 --- a/tests/test_paths.toml +++ b/tests/test_paths.toml @@ -219,10 +219,10 @@ code = ''' assert(info.type == LFS_TYPE_DIR); assert(info.size == 0); - lfsr_mkdir(&lfs, "/") => LFS_ERR_INVAL; + lfsr_mkdir(&lfs, "/") => LFS_ERR_EXIST; lfsr_file_t file; lfsr_file_open(&lfs, &file, "/", - LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_INVAL; + LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR; lfsr_remove(&lfs, "/") => LFS_ERR_INVAL; lfsr_unmount(&lfs) => 0;