Github/littlefs
1
Fork 0
Code Actions Activity

Added some corner-case tests, fixed related bugs/POSIX nuances

Browse Source 
POSIX is notoriously full of subtle and confusing nuances. Not through
any fault of POSIX, but as a result of trying to describe a complex
system with simple and easy to use operations.

Corner cases fixed here:

- rename("dir", "file") => ENOTDIR

  This is the main surprise to me, and a mistake on my part. I thought
  EISDIR would be appropriate for any renames with mismatched types,
  since both involve a directory. It would be simpler code-wise, and
  avoid ambiguity around if "file" is not a dir, or some other file
  exists in the file's path. But I guess ENOTDIR makes more sense if you
  think of the destination as the target being operated on.

- remove("/") => EINVAL
- rename("/", "x") => EINVAL
- rename("x", "/") => ENOTEMPTY
- open("/") => EISDIR

  It's a bit difficult to lookup what error codes around root operations
  should be, since they mostly end up as EPERM on modern systems, but
  this doesn't really make sense for littlefs.

  The solution chosen here is to prefer directory-related errors (EISDIR,
  ENOTEMPTY) when possible, and fall back to EINVAL when the only issue
  is that the target is the root directory.

Also I tweaked lfsr_mtree_pathlookup a bit so mid=0 indicates the target
is the root and mid=-1 indicates the target can't be created (because of
a missing directory). I think using mid=0 for the latter is a leftover
from when mid=-1 was a bit of a mess...
This commit is contained in:
Christopher Haster
2023-12-01 00:31:19 -06:00
parent abbd2d6c3f
commit 939dd2145a
3 changed files with 540 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_dirs.toml
+2 -4
View File
@@ -281,16 +281,14 @@ code = '''
}
// try to make root, which doesn't make sense
err = lfsr_mkdir(&lfs, "/");
assert(err == LFS_ERR_EXIST || err == 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
err = lfsr_mkdir(&lfs, "/");
assert(err == LFS_ERR_EXIST || err == LFS_ERR_INVAL);
lfsr_mkdir(&lfs, "/") => LFS_ERR_EXIST;
// remount?
if (REMOUNT) {
tests/test_files.toml
+505 -7
View File
@@ -288,7 +288,7 @@ code = '''
// try to rename a directory onto our file
lfsr_mkdir(&lfs, "not_hello") => 0;
lfsr_rename(&lfs, "not_hello", "hello") => LFS_ERR_ISDIR;
lfsr_rename(&lfs, "not_hello", "hello") => LFS_ERR_NOTDIR;
// remount?
if (REMOUNT) {
@@ -426,16 +426,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",
@@ -446,7 +446,82 @@ code = '''
lfsr_file_write(&lfs, &file, wbuf, wsize) => wsize;
lfsr_file_close(&lfs, &file) => 0;
lfsr_rename(&lfs, "not_hello", "/") => LFS_ERR_INVAL;
lfsr_rename(&lfs, "not_hello", "/") => LFS_ERR_ISDIR;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// check our root with stat
struct lfs_info info;
lfsr_stat(&lfs, "/", &info) => 0;
assert(strcmp(info.name, "/") == 0);
assert(info.type == LFS_TYPE_DIR);
// and with dir read
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "not_hello") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == wsize);
lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT;
lfsr_dir_close(&lfs, &dir) => 0;
// did we corrupt our renaming file?
// try reading our file
lfsr_file_open(&lfs, &file, "not_hello", LFS_O_RDONLY) => 0;
// is size correct?
lfsr_file_size(&lfs, &file) => wsize;
// try reading
uint8_t rbuf[8192];
memset(rbuf, 0xaa, sizeof(rbuf));
lfsr_file_read(&lfs, &file, rbuf, sizeof(rbuf)) => wsize;
assert(memcmp(rbuf, wbuf, wsize) == 0);
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
'''
# an invalid path is also not a file (kind of?)
[cases.test_files_noent_not_file]
defines.REMOUNT = [false, true]
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// try reading our invalid path as a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "no/hello", LFS_O_RDONLY) => LFS_ERR_NOENT;
// try writing our root as a file
lfsr_file_open(&lfs, &file, "no/hello", LFS_O_WRONLY) => LFS_ERR_NOENT;
lfsr_file_open(&lfs, &file, "no/hello",
LFS_O_WRONLY | LFS_O_TRUNC) => LFS_ERR_NOENT;
lfsr_file_open(&lfs, &file, "no/hello",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOENT;
lfsr_file_open(&lfs, &file, "no/hello",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_TRUNC) => LFS_ERR_NOENT;
// try rename a file on top of our invalid path
lfsr_file_open(&lfs, &file, "not_hello",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
uint8_t wbuf[8192];
strcpy((char*)wbuf, "Hello World!");
lfs_size_t wsize = strlen((const char*)wbuf);
lfsr_file_write(&lfs, &file, wbuf, wsize) => wsize;
lfsr_file_close(&lfs, &file) => 0;
lfsr_rename(&lfs, "not_hello", "no/hello") => LFS_ERR_NOENT;
// remount?
if (REMOUNT) {
@@ -838,6 +913,429 @@ code = '''
lfsr_unmount(&lfs) => 0;
'''
# test renaming a file to itself
[cases.test_files_mv_noop]
defines.SIZE = [
'0',
'CACHE_SIZE/2',
'2*CACHE_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
defines.REMOUNT = [false, true]
defines.CACHE_SIZE = 64
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_WRONLY | LFS_O_CREAT) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, wbuf, SIZE) => SIZE;
lfsr_file_close(&lfs, &file) => 0;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// rename the file
lfsr_rename(&lfs, "amethyst", "amethyst") => 0;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// check our file with stat
struct lfs_info info;
lfsr_stat(&lfs, "amethyst", &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
// and with dir read
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT;
lfsr_dir_close(&lfs, &dir) => 0;
// try reading our file
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_RDONLY) => 0;
// is size correct?
lfsr_file_size(&lfs, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfsr_file_read(&lfs, &file, rbuf, 2*SIZE) => SIZE;
assert(memcmp(rbuf, wbuf, SIZE) == 0);
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
'''
# test renaming a file onto a dir
[cases.test_files_mv_not_file]
defines.SIZE = [
'0',
'CACHE_SIZE/2',
'2*CACHE_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
defines.REMOUNT = [false, true]
defines.CACHE_SIZE = 64
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_WRONLY | LFS_O_CREAT) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, wbuf, SIZE) => SIZE;
lfsr_file_close(&lfs, &file) => 0;
// create a dir
lfsr_mkdir(&lfs, "basalt") => 0;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// rename the file
lfsr_rename(&lfs, "amethyst", "basalt") => LFS_ERR_ISDIR;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// check that nothing changed in our file/dir with stat
struct lfs_info info;
lfsr_stat(&lfs, "amethyst", &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_stat(&lfs, "basalt", &info) => 0;
assert(strcmp(info.name, "basalt") == 0);
assert(info.type == LFS_TYPE_DIR);
// and with dir read
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "basalt") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT;
lfsr_dir_close(&lfs, &dir) => 0;
// try reading our file
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_RDONLY) => 0;
// is size correct?
lfsr_file_size(&lfs, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfsr_file_read(&lfs, &file, rbuf, 2*SIZE) => SIZE;
assert(memcmp(rbuf, wbuf, SIZE) == 0);
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
'''
# test renaming a dir onto a file
[cases.test_files_mv_not_dir]
defines.SIZE = [
'0',
'CACHE_SIZE/2',
'2*CACHE_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
defines.REMOUNT = [false, true]
defines.CACHE_SIZE = 64
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_WRONLY | LFS_O_CREAT) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, wbuf, SIZE) => SIZE;
lfsr_file_close(&lfs, &file) => 0;
// create a dir
lfsr_mkdir(&lfs, "basalt") => 0;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// rename the dir
lfsr_rename(&lfs, "basalt", "amethyst") => LFS_ERR_NOTDIR;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// check that nothing changed in our file/dir with stat
struct lfs_info info;
lfsr_stat(&lfs, "amethyst", &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_stat(&lfs, "basalt", &info) => 0;
assert(strcmp(info.name, "basalt") == 0);
assert(info.type == LFS_TYPE_DIR);
// and with dir read
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "basalt") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT;
lfsr_dir_close(&lfs, &dir) => 0;
// try reading our file
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_RDONLY) => 0;
// is size correct?
lfsr_file_size(&lfs, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfsr_file_read(&lfs, &file, rbuf, 2*SIZE) => SIZE;
assert(memcmp(rbuf, wbuf, SIZE) == 0);
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
'''
# test renaming a file onto root
[cases.test_files_mv_not_root]
defines.SIZE = [
'0',
'CACHE_SIZE/2',
'2*CACHE_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
defines.REMOUNT = [false, true]
defines.CACHE_SIZE = 64
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_WRONLY | LFS_O_CREAT) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, wbuf, SIZE) => SIZE;
lfsr_file_close(&lfs, &file) => 0;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// rename the file
lfsr_rename(&lfs, "amethyst", "/") => LFS_ERR_ISDIR;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// check that nothing changed in our file/dir with stat
struct lfs_info info;
lfsr_stat(&lfs, "amethyst", &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
// and with dir read
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT;
lfsr_dir_close(&lfs, &dir) => 0;
// try reading our file
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_RDONLY) => 0;
// is size correct?
lfsr_file_size(&lfs, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfsr_file_read(&lfs, &file, rbuf, 2*SIZE) => SIZE;
assert(memcmp(rbuf, wbuf, SIZE) == 0);
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
'''
# test renaming a file onto an invalid path
[cases.test_files_mv_not_noent]
defines.SIZE = [
'0',
'CACHE_SIZE/2',
'2*CACHE_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
defines.REMOUNT = [false, true]
defines.CACHE_SIZE = 64
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_WRONLY | LFS_O_CREAT) => 0;
uint8_t wbuf[SIZE];
uint32_t prng = 42;
for (lfs_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, wbuf, SIZE) => SIZE;
lfsr_file_close(&lfs, &file) => 0;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// rename the file
lfsr_rename(&lfs, "amethyst", "no/amethyst") => LFS_ERR_NOENT;
// remount?
if (REMOUNT) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, CFG) => 0;
}
// check that nothing changed in our file/dir with stat
struct lfs_info info;
lfsr_stat(&lfs, "amethyst", &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
// and with dir read
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "amethyst") == 0);
assert(info.type == LFS_TYPE_REG);
assert(info.size == SIZE);
lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT;
lfsr_dir_close(&lfs, &dir) => 0;
// try reading our file
lfsr_file_open(&lfs, &file, "amethyst", LFS_O_RDONLY) => 0;
// is size correct?
lfsr_file_size(&lfs, &file) => SIZE;
// try reading
uint8_t rbuf[2*SIZE];
memset(rbuf, 0xaa, 2*SIZE);
lfsr_file_read(&lfs, &file, rbuf, 2*SIZE) => SIZE;
assert(memcmp(rbuf, wbuf, SIZE) == 0);
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
'''
# TODO
# [cases.test_files_rm]