Skip to content

Fix block-boundary truncate issues #800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
geky merged 5 commits into from
Apr 26, 2023
Merged

Fix block-boundary truncate issues #800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7b151e1
Add test scenario for truncating to a block size
colin-foster-in-advantage Jan 26, 2023
d5dc487
Expanded truncate tests to test more corner cases
geky Apr 17, 2023
6dc18c3
Fixed block-boundary truncate issue
geky Apr 17, 2023
e57402c
Added ability to revert to inline file in lfs_file_truncate
geky Apr 17, 2023
922a35b
Merge remote-tracking branch 'origin/devel' into fix-boundary-truncates
geky Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions lfs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3465,7 +3465,7 @@ static lfs_ssize_t lfs_file_flushedwrite(lfs_t *lfs, lfs_file_t *file,
// find out which block we're extending from
int err = lfs_ctz_find(lfs, NULL, &file->cache,
file->ctz.head, file->ctz.size,
file->pos-1, &file->block, &file->off);
file->pos-1, &file->block, &(lfs_off_t){0});
if (err) {
file->flags |= LFS_F_ERRED;
return err;
Expand Down Expand Up @@ -3643,26 +3643,55 @@ static int lfs_file_rawtruncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) {
lfs_off_t pos = file->pos;
lfs_off_t oldsize = lfs_file_rawsize(lfs, file);
if (size < oldsize) {
// need to flush since directly changing metadata
int err = lfs_file_flush(lfs, file);
if (err) {
return err;
}
// revert to inline file?
if (size <= lfs_min(0x3fe, lfs_min(
lfs->cfg->cache_size,
(lfs->cfg->metadata_max ?
lfs->cfg->metadata_max : lfs->cfg->block_size) / 8))) {
// flush+seek to head
lfs_soff_t res = lfs_file_rawseek(lfs, file, 0, LFS_SEEK_SET);
if (res < 0) {
return (int)res;
}

// lookup new head in ctz skip list
err = lfs_ctz_find(lfs, NULL, &file->cache,
file->ctz.head, file->ctz.size,
size, &file->block, &file->off);
if (err) {
return err;
}
// read our data into rcache temporarily
lfs_cache_drop(lfs, &lfs->rcache);
res = lfs_file_flushedread(lfs, file,
lfs->rcache.buffer, size);
if (res < 0) {
return (int)res;
}

// need to set pos/block/off consistently so seeking back to
// the old position does not get confused
file->pos = size;
file->ctz.head = file->block;
file->ctz.size = size;
file->flags |= LFS_F_DIRTY | LFS_F_READING;
file->ctz.head = LFS_BLOCK_INLINE;
file->ctz.size = size;
file->flags |= LFS_F_DIRTY | LFS_F_READING | LFS_F_INLINE;
file->cache.block = file->ctz.head;
file->cache.off = 0;
file->cache.size = lfs->cfg->cache_size;
memcpy(file->cache.buffer, lfs->rcache.buffer, size);

} else {
// need to flush since directly changing metadata
int err = lfs_file_flush(lfs, file);
if (err) {
return err;
}

// lookup new head in ctz skip list
err = lfs_ctz_find(lfs, NULL, &file->cache,
file->ctz.head, file->ctz.size,
size-1, &file->block, &(lfs_off_t){0});
if (err) {
return err;
}

// need to set pos/block/off consistently so seeking back to
// the old position does not get confused
file->pos = size;
file->ctz.head = file->block;
file->ctz.size = size;
file->flags |= LFS_F_DIRTY | LFS_F_READING;
}
} else if (size > oldsize) {
// flush+seek if not already at end
lfs_soff_t res = lfs_file_rawseek(lfs, file, 0, LFS_SEEK_END);
Expand Down
90 changes: 56 additions & 34 deletions tests/test_truncate.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# simple truncate
[cases.test_truncate_simple]
defines.MEDIUMSIZE = [32, 2048]
defines.LARGESIZE = 8192
defines.MEDIUMSIZE = [31, 32, 33, 511, 512, 513, 2047, 2048, 2049]
defines.LARGESIZE = [32, 33, 512, 513, 2048, 2049, 8192, 8193]
if = 'MEDIUMSIZE < LARGESIZE'
code = '''
lfs_t lfs;
lfs_format(&lfs, cfg) => 0;
Expand All @@ -14,7 +15,8 @@ code = '''
strcpy((char*)buffer, "hair");
size_t size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < LARGESIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, LARGESIZE-j))
=> lfs_min(size, LARGESIZE-j);
}
lfs_file_size(&lfs, &file) => LARGESIZE;

Expand All @@ -37,8 +39,9 @@ code = '''

size = strlen("hair");
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_read(&lfs, &file, buffer, size) => size;
memcmp(buffer, "hair", size) => 0;
lfs_file_read(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
memcmp(buffer, "hair", lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_read(&lfs, &file, buffer, size) => 0;

Expand All @@ -48,8 +51,9 @@ code = '''

# truncate and read
[cases.test_truncate_read]
defines.MEDIUMSIZE = [32, 2048]
defines.LARGESIZE = 8192
defines.MEDIUMSIZE = [31, 32, 33, 511, 512, 513, 2047, 2048, 2049]
defines.LARGESIZE = [32, 33, 512, 513, 2048, 2049, 8192, 8193]
if = 'MEDIUMSIZE < LARGESIZE'
code = '''
lfs_t lfs;
lfs_format(&lfs, cfg) => 0;
Expand All @@ -62,7 +66,8 @@ code = '''
strcpy((char*)buffer, "hair");
size_t size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < LARGESIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, LARGESIZE-j))
=> lfs_min(size, LARGESIZE-j);
}
lfs_file_size(&lfs, &file) => LARGESIZE;

Expand All @@ -78,8 +83,9 @@ code = '''

size = strlen("hair");
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_read(&lfs, &file, buffer, size) => size;
memcmp(buffer, "hair", size) => 0;
lfs_file_read(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
memcmp(buffer, "hair", lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_read(&lfs, &file, buffer, size) => 0;

Expand All @@ -92,8 +98,9 @@ code = '''

size = strlen("hair");
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_read(&lfs, &file, buffer, size) => size;
memcmp(buffer, "hair", size) => 0;
lfs_file_read(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
memcmp(buffer, "hair", lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_read(&lfs, &file, buffer, size) => 0;

Expand Down Expand Up @@ -148,7 +155,7 @@ code = '''
lfs_file_truncate(&lfs, &file, trunc) => 0;
lfs_file_tell(&lfs, &file) => qsize;
lfs_file_size(&lfs, &file) => trunc;

/* Read should produce second quarter */
lfs_file_read(&lfs, &file, rb, size) => trunc - qsize;
memcmp(rb, wb + qsize, trunc - qsize) => 0;
Expand All @@ -159,8 +166,9 @@ code = '''

# truncate and write
[cases.test_truncate_write]
defines.MEDIUMSIZE = [32, 2048]
defines.LARGESIZE = 8192
defines.MEDIUMSIZE = [31, 32, 33, 511, 512, 513, 2047, 2048, 2049]
defines.LARGESIZE = [32, 33, 512, 513, 2048, 2049, 8192, 8193]
if = 'MEDIUMSIZE < LARGESIZE'
code = '''
lfs_t lfs;
lfs_format(&lfs, cfg) => 0;
Expand All @@ -173,7 +181,8 @@ code = '''
strcpy((char*)buffer, "hair");
size_t size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < LARGESIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, LARGESIZE-j))
=> lfs_min(size, LARGESIZE-j);
}
lfs_file_size(&lfs, &file) => LARGESIZE;

Expand All @@ -184,13 +193,16 @@ code = '''
lfs_file_open(&lfs, &file, "baldywrite", LFS_O_RDWR) => 0;
lfs_file_size(&lfs, &file) => LARGESIZE;

/* truncate */
lfs_file_truncate(&lfs, &file, MEDIUMSIZE) => 0;
lfs_file_size(&lfs, &file) => MEDIUMSIZE;

/* and write */
strcpy((char*)buffer, "bald");
size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
}
lfs_file_size(&lfs, &file) => MEDIUMSIZE;

Expand All @@ -203,8 +215,9 @@ code = '''

size = strlen("bald");
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_read(&lfs, &file, buffer, size) => size;
memcmp(buffer, "bald", size) => 0;
lfs_file_read(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
memcmp(buffer, "bald", lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_read(&lfs, &file, buffer, size) => 0;

Expand All @@ -215,7 +228,7 @@ code = '''
# truncate write under powerloss
[cases.test_truncate_reentrant_write]
defines.SMALLSIZE = [4, 512]
defines.MEDIUMSIZE = [32, 1024]
defines.MEDIUMSIZE = [0, 3, 4, 5, 31, 32, 33, 511, 512, 513, 1023, 1024, 1025]
defines.LARGESIZE = 2048
reentrant = true
code = '''
Expand All @@ -236,10 +249,11 @@ code = '''
size == (size_t)SMALLSIZE);
for (lfs_off_t j = 0; j < size; j += 4) {
uint8_t buffer[1024];
lfs_file_read(&lfs, &file, buffer, 4) => 4;
assert(memcmp(buffer, "hair", 4) == 0 ||
memcmp(buffer, "bald", 4) == 0 ||
memcmp(buffer, "comb", 4) == 0);
lfs_file_read(&lfs, &file, buffer, lfs_min(4, size-j))
=> lfs_min(4, size-j);
assert(memcmp(buffer, "hair", lfs_min(4, size-j)) == 0 ||
memcmp(buffer, "bald", lfs_min(4, size-j)) == 0 ||
memcmp(buffer, "comb", lfs_min(4, size-j)) == 0);
}
lfs_file_close(&lfs, &file) => 0;
}
Expand All @@ -251,19 +265,23 @@ code = '''
strcpy((char*)buffer, "hair");
size_t size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < LARGESIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, LARGESIZE-j))
=> lfs_min(size, LARGESIZE-j);
}
lfs_file_size(&lfs, &file) => LARGESIZE;
lfs_file_close(&lfs, &file) => 0;

lfs_file_open(&lfs, &file, "baldy", LFS_O_RDWR) => 0;
lfs_file_size(&lfs, &file) => LARGESIZE;
/* truncate */
lfs_file_truncate(&lfs, &file, MEDIUMSIZE) => 0;
lfs_file_size(&lfs, &file) => MEDIUMSIZE;
/* and write */
strcpy((char*)buffer, "bald");
size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
}
lfs_file_size(&lfs, &file) => MEDIUMSIZE;
lfs_file_close(&lfs, &file) => 0;
Expand All @@ -275,7 +293,8 @@ code = '''
strcpy((char*)buffer, "comb");
size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < SMALLSIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, SMALLSIZE-j))
=> lfs_min(size, SMALLSIZE-j);
}
lfs_file_size(&lfs, &file) => SMALLSIZE;
lfs_file_close(&lfs, &file) => 0;
Expand Down Expand Up @@ -429,7 +448,7 @@ code = '''

# noop truncate
[cases.test_truncate_nop]
defines.MEDIUMSIZE = [32, 2048]
defines.MEDIUMSIZE = [32, 33, 512, 513, 2048, 2049, 8192, 8193]
code = '''
lfs_t lfs;
lfs_format(&lfs, cfg) => 0;
Expand All @@ -442,10 +461,11 @@ code = '''
strcpy((char*)buffer, "hair");
size_t size = strlen((char*)buffer);
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_write(&lfs, &file, buffer, size) => size;
lfs_file_write(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);

// this truncate should do nothing
lfs_file_truncate(&lfs, &file, j+size) => 0;
lfs_file_truncate(&lfs, &file, j+lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_size(&lfs, &file) => MEDIUMSIZE;

Expand All @@ -455,8 +475,9 @@ code = '''
lfs_file_size(&lfs, &file) => MEDIUMSIZE;

for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_read(&lfs, &file, buffer, size) => size;
memcmp(buffer, "hair", size) => 0;
lfs_file_read(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
memcmp(buffer, "hair", lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_read(&lfs, &file, buffer, size) => 0;

Expand All @@ -468,8 +489,9 @@ code = '''
lfs_file_open(&lfs, &file, "baldynoop", LFS_O_RDWR) => 0;
lfs_file_size(&lfs, &file) => MEDIUMSIZE;
for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
lfs_file_read(&lfs, &file, buffer, size) => size;
memcmp(buffer, "hair", size) => 0;
lfs_file_read(&lfs, &file, buffer, lfs_min(size, MEDIUMSIZE-j))
=> lfs_min(size, MEDIUMSIZE-j);
memcmp(buffer, "hair", lfs_min(size, MEDIUMSIZE-j)) => 0;
}
lfs_file_read(&lfs, &file, buffer, size) => 0;
lfs_file_close(&lfs, &file) => 0;
Expand Down