Fixed issue where FBIG errors did not set the DESYNC flag

I think the assumption was that since these errors are trivially noops,
they shouldn't change any file state. But this doesn't match the
behavior of other errors, which is inconsistent and probably not what
users expect.

Also added a couple tests around FBIG that should catch this in the
future.

Curiously this actually saved a word of code, I guess because of
rerouting all errors through the same function epilogues:

           code          stack
  before: 36416           2616
  after:  36412 (-0.0%)   2616 (+0.0%)
This commit is contained in:
Christopher Haster
2024-08-20 15:15:48 -05:00
parent ea017d33fe
commit da9ac39c88
2 changed files with 338 additions and 20 deletions
+25 -20
View File
@@ -12402,11 +12402,6 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
// can't write to readonly files
LFS_ASSERT(!lfsr_o_isrdonly(file->o.o.flags));
// would this write make our file larger than our file limit?
if (size > lfs->file_limit - file->pos) {
return LFS_ERR_FBIG;
}
// size=0 is a bit special and is guaranteed to have no effects on the
// underlying file, this means no updating file pos or file size
//
@@ -12415,6 +12410,13 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
return 0;
}
// would this write make our file larger than our file limit?
int err;
if (size > lfs->file_limit - file->pos) {
err = LFS_ERR_FBIG;
goto failed;
}
// clobber entangled traversals
lfsr_omdir_mkdirty(lfs, &file->o.o);
// checkpoint the allocator
@@ -12441,7 +12443,6 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
const uint8_t *buffer_ = buffer;
lfs_size_t written = 0;
int err;
while (size > 0) {
// bypass buffer?
//
@@ -12609,8 +12610,10 @@ failed:;
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
LFS_ASSERT(lfsr_omdir_isopen(lfs, &file->o.o));
// removed? we can't sync
int err;
if (lfsr_o_iszombie(file->o.o.flags)) {
return LFS_ERR_NOENT;
err = LFS_ERR_NOENT;
goto failed;
}
// first flush any data in our buffer, this is a noop if already
@@ -12620,7 +12623,7 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// flush succeeds but mdir commit fails it's ok to fall back to
// our flushed state
//
int err = lfsr_file_flush(lfs, file);
err = lfsr_file_flush(lfs, file);
if (err) {
goto failed;
}
@@ -12840,17 +12843,19 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
// can't write to readonly files
LFS_ASSERT(!lfsr_o_isrdonly(file->o.o.flags));
// exceeds our file limit?
if (size_ > lfs->file_limit) {
return LFS_ERR_FBIG;
}
// do nothing if our size does not change
lfs_off_t size = lfsr_file_size_(file);
if (lfsr_file_size_(file) == size_) {
return 0;
}
// exceeds our file limit?
int err;
if (size_ > lfs->file_limit) {
err = LFS_ERR_FBIG;
goto failed;
}
// clobber entangled traversals
lfsr_omdir_mkdirty(lfs, &file->o.o);
// checkpoint the allocator
@@ -12859,7 +12864,6 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
file->o.o.flags |= LFS_O_UNSYNC;
// does our file become small?
int err;
if (size_ <= lfsr_file_inlinesize(lfs, file)) {
// if our data is not already in our buffer we unfortunately
// need to flush so our buffer is available to hold everything
@@ -12948,17 +12952,19 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
// can't write to readonly files
LFS_ASSERT(!lfsr_o_isrdonly(file->o.o.flags));
// exceeds our file limit?
if (size_ > lfs->file_limit) {
return LFS_ERR_FBIG;
}
// do nothing if our size does not change
lfs_off_t size = lfsr_file_size_(file);
if (size == size_) {
return 0;
}
// exceeds our file limit?
int err;
if (size_ > lfs->file_limit) {
err = LFS_ERR_FBIG;
goto failed;
}
// clobber entangled traversals
lfsr_omdir_mkdirty(lfs, &file->o.o);
// checkpoint the allocator
@@ -12967,7 +12973,6 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
file->o.o.flags |= LFS_O_UNSYNC;
// does our file become small?
int err;
if (size_ <= lfsr_file_inlinesize(lfs, file)) {
// if our data is not already in our buffer we unfortunately
// need to flush so our buffer is available to hold everything
+313
View File
@@ -2892,6 +2892,8 @@ code = '''
'''
# test other corner conditions
# test that seeking to a negative offset errors
[cases.test_fwrite_seek_negative]
defines.WHENCE = ['LFS_SEEK_SET', 'LFS_SEEK_CUR', 'LFS_SEEK_END']
defines.SIZE = [
@@ -2999,6 +3001,317 @@ code = '''
lfsr_unmount(&lfs) => 0;
'''
# test that write overflow errors
[cases.test_fwrite_fbig]
defines.SIZE = [
'FILE_BUFFER_SIZE/2',
'2*FILE_BUFFER_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
# INIT=0 => no init
# INIT=1 => fill with data
# INIT=2 => truncate to size
defines.INIT = [0, 1, 2]
defines.MODE = ['LFS_O_WRONLY', 'LFS_O_RDWR']
if = [
# this just saves testing time
'SIZE <= 4*1024*FRAGMENT_SIZE',
]
code = '''
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "hello",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
// simulate our file in ram
uint8_t sim[SIZE];
lfs_off_t size;
uint32_t prng = 42;
if (INIT == 0) {
memset(sim, 0, SIZE);
size = 0;
} else if (INIT == 1) {
for (lfs_size_t i = 0; i < SIZE; i++) {
sim[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, sim, SIZE) => SIZE;
size = SIZE;
} else {
memset(sim, 0, SIZE);
lfsr_file_truncate(&lfs, &file, SIZE) => 0;
size = SIZE;
}
lfsr_file_close(&lfs, &file) => 0;
// seek to near the file limit
lfsr_file_open(&lfs, &file, "hello", MODE) => 0;
lfsr_file_seek(&lfs, &file, LFS_FILE_MAX-(SIZE/2), LFS_SEEK_SET)
=> LFS_FILE_MAX-(SIZE/2);
// try to write past the file limit, this should fail
uint8_t wbuf[SIZE];
for (lfs_size_t i = 0; i < SIZE; i++) {
wbuf[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, wbuf, SIZE) => LFS_ERR_FBIG;
lfsr_file_close(&lfs, &file) => 0;
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
}
// check our file with stat
struct lfs_info info;
lfsr_stat(&lfs, "hello", &info) => 0;
assert(strcmp(info.name, "hello") == 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);
assert(info.size == 0);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
assert(info.size == 0);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "hello") == 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, "hello", 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;
// does our file match our simulation?
assert(memcmp(rbuf, sim, size) == 0);
lfsr_file_close(&lfs, &file) => 0;
}
lfsr_unmount(&lfs) => 0;
'''
# test that truncate overflow errors
[cases.test_fwrite_truncate_fbig]
defines.SIZE = [
'FILE_BUFFER_SIZE/2',
'2*FILE_BUFFER_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
# INIT=0 => no init
# INIT=1 => fill with data
# INIT=2 => truncate to size
defines.INIT = [0, 1, 2]
defines.MODE = ['LFS_O_WRONLY', 'LFS_O_RDWR']
if = [
# this just saves testing time
'SIZE <= 4*1024*FRAGMENT_SIZE',
]
code = '''
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "hello",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
// simulate our file in ram
uint8_t sim[SIZE];
lfs_off_t size;
uint32_t prng = 42;
if (INIT == 0) {
memset(sim, 0, SIZE);
size = 0;
} else if (INIT == 1) {
for (lfs_size_t i = 0; i < SIZE; i++) {
sim[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, sim, SIZE) => SIZE;
size = SIZE;
} else {
memset(sim, 0, SIZE);
lfsr_file_truncate(&lfs, &file, SIZE) => 0;
size = SIZE;
}
lfsr_file_close(&lfs, &file) => 0;
// try to truncate the file past the file limit, this should fail
lfsr_file_open(&lfs, &file, "hello", MODE) => 0;
lfsr_file_truncate(&lfs, &file, LFS_FILE_MAX+(SIZE/2)) => LFS_ERR_FBIG;
lfsr_file_close(&lfs, &file) => 0;
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
}
// check our file with stat
struct lfs_info info;
lfsr_stat(&lfs, "hello", &info) => 0;
assert(strcmp(info.name, "hello") == 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);
assert(info.size == 0);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
assert(info.size == 0);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "hello") == 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, "hello", 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;
// does our file match our simulation?
assert(memcmp(rbuf, sim, size) == 0);
lfsr_file_close(&lfs, &file) => 0;
}
lfsr_unmount(&lfs) => 0;
'''
# test that fruncate overflow errors
[cases.test_fwrite_fruncate_fbig]
defines.SIZE = [
'FILE_BUFFER_SIZE/2',
'2*FILE_BUFFER_SIZE',
'BLOCK_SIZE/2',
'BLOCK_SIZE',
'2*BLOCK_SIZE',
'4*BLOCK_SIZE',
]
# INIT=0 => no init
# INIT=1 => fill with data
# INIT=2 => truncate to size
defines.INIT = [0, 1, 2]
defines.MODE = ['LFS_O_WRONLY', 'LFS_O_RDWR']
if = [
# this just saves testing time
'SIZE <= 4*1024*FRAGMENT_SIZE',
]
code = '''
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
// create a file
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "hello",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
// simulate our file in ram
uint8_t sim[SIZE];
lfs_off_t size;
uint32_t prng = 42;
if (INIT == 0) {
memset(sim, 0, SIZE);
size = 0;
} else if (INIT == 1) {
for (lfs_size_t i = 0; i < SIZE; i++) {
sim[i] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfsr_file_write(&lfs, &file, sim, SIZE) => SIZE;
size = SIZE;
} else {
memset(sim, 0, SIZE);
lfsr_file_truncate(&lfs, &file, SIZE) => 0;
size = SIZE;
}
lfsr_file_close(&lfs, &file) => 0;
// try to truncate the file past the file limit, this should fail
lfsr_file_open(&lfs, &file, "hello", MODE) => 0;
lfsr_file_fruncate(&lfs, &file, LFS_FILE_MAX+(SIZE/2)) => LFS_ERR_FBIG;
lfsr_file_close(&lfs, &file) => 0;
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfsr_unmount(&lfs) => 0;
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
}
// check our file with stat
struct lfs_info info;
lfsr_stat(&lfs, "hello", &info) => 0;
assert(strcmp(info.name, "hello") == 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);
assert(info.size == 0);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "..") == 0);
assert(info.type == LFS_TYPE_DIR);
assert(info.size == 0);
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, "hello") == 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, "hello", 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;
// does our file match our simulation?
assert(memcmp(rbuf, sim, size) == 0);
lfsr_file_close(&lfs, &file) => 0;
}
lfsr_unmount(&lfs) => 0;
'''
# heavy fuzz test with rw seeks, truncate, and fruncate
[cases.test_fwrite_rwtf_fuzz]
defines.N = 20