Skip to content

Commit

Permalink
Fixed issue updating dir struct when extended dir chain
Browse files Browse the repository at this point in the history
Like most of the lfs_dir_t functions, lfs_dir_append is responsible for
updating the lfs_dir_t struct if the underlying directory block is
moved. This property makes handling worn out blocks much easier by
removing the amount of state that needs to be considered during a
directory update.

However, extending the dir chain is a bit of a corner case. It's not
changing the old block, but callers of lfs_dir_append do assume the
"entry" will reside in "dir" after lfs_dir_append completes.

This issue only occurs when creating files, since mkdir does not use
the entry after lfs_dir_append. Unfortunately, the tests against
extending the directory chain were all made using mkdir.

Found by schouleu
  • Loading branch information
geky committed Mar 1, 2018
1 parent d9c3637 commit 9ee112a
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 10 deletions.
20 changes: 10 additions & 10 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -658,17 +658,17 @@ static int lfs_dir_append(lfs_t *lfs, lfs_dir_t *dir,

// we need to allocate a new dir block
if (!(0x80000000 & dir->d.size)) {
lfs_dir_t newdir;
int err = lfs_dir_alloc(lfs, &newdir);
lfs_dir_t olddir = *dir;
int err = lfs_dir_alloc(lfs, dir);
if (err) {
return err;
}

newdir.d.tail[0] = dir->d.tail[0];
newdir.d.tail[1] = dir->d.tail[1];
entry->off = newdir.d.size - 4;
dir->d.tail[0] = olddir.d.tail[0];
dir->d.tail[1] = olddir.d.tail[1];
entry->off = dir->d.size - 4;
lfs_entry_tole32(&entry->d);
err = lfs_dir_commit(lfs, &newdir, (struct lfs_region[]){
err = lfs_dir_commit(lfs, dir, (struct lfs_region[]){
{entry->off, 0, &entry->d, sizeof(entry->d)},
{entry->off, 0, data, entry->d.nlen}
}, 2);
Expand All @@ -677,10 +677,10 @@ static int lfs_dir_append(lfs_t *lfs, lfs_dir_t *dir,
return err;
}

dir->d.size |= 0x80000000;
dir->d.tail[0] = newdir.pair[0];
dir->d.tail[1] = newdir.pair[1];
return lfs_dir_commit(lfs, dir, NULL, 0);
olddir.d.size |= 0x80000000;
olddir.d.tail[0] = dir->pair[0];
olddir.d.tail[1] = dir->pair[1];
return lfs_dir_commit(lfs, &olddir, NULL, 0);
}

int err = lfs_dir_fetch(lfs, dir, dir->d.tail);
Expand Down
66 changes: 66 additions & 0 deletions tests/test_dirs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ tests/test.py << TEST
sprintf((char*)buffer, "test%d", i);
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, (char*)buffer) => 0;
info.type => LFS_TYPE_DIR;
}
lfs_dir_read(&lfs, &dir[0], &info) => 0;
lfs_unmount(&lfs) => 0;
Expand Down Expand Up @@ -355,5 +356,70 @@ tests/test.py << TEST
lfs_unmount(&lfs) => 0;
TEST

echo "--- Multi-block directory with files ---"
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_mkdir(&lfs, "prickly-pear") => 0;
for (int i = 0; i < $LARGESIZE; i++) {
sprintf((char*)buffer, "prickly-pear/test%d", i);
lfs_file_open(&lfs, &file[0], (char*)buffer,
LFS_O_WRONLY | LFS_O_CREAT) => 0;
size = 6;
memcpy(wbuffer, "Hello", size);
lfs_file_write(&lfs, &file[0], wbuffer, size) => size;
lfs_file_close(&lfs, &file[0]) => 0;
}
lfs_unmount(&lfs) => 0;
TEST
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "prickly-pear") => 0;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, ".") => 0;
info.type => LFS_TYPE_DIR;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "..") => 0;
info.type => LFS_TYPE_DIR;
for (int i = 0; i < $LARGESIZE; i++) {
sprintf((char*)buffer, "test%d", i);
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, (char*)buffer) => 0;
info.type => LFS_TYPE_REG;
info.size => 6;
}
lfs_dir_read(&lfs, &dir[0], &info) => 0;
lfs_unmount(&lfs) => 0;
TEST

echo "--- Multi-block remove with files ---"
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_remove(&lfs, "prickly-pear") => LFS_ERR_NOTEMPTY;
for (int i = 0; i < $LARGESIZE; i++) {
sprintf((char*)buffer, "prickly-pear/test%d", i);
lfs_remove(&lfs, (char*)buffer) => 0;
}
lfs_remove(&lfs, "prickly-pear") => 0;
lfs_unmount(&lfs) => 0;
TEST
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "/") => 0;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, ".") => 0;
info.type => LFS_TYPE_DIR;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "..") => 0;
info.type => LFS_TYPE_DIR;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "burito") => 0;
info.type => LFS_TYPE_REG;
lfs_dir_read(&lfs, &dir[0], &info) => 0;
lfs_dir_close(&lfs, &dir[0]) => 0;
lfs_unmount(&lfs) => 0;
TEST

echo "--- Results ---"
tests/stats.py
19 changes: 19 additions & 0 deletions tests/test_files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,24 @@ tests/test.py << TEST
lfs_unmount(&lfs) => 0;
TEST

echo "--- Many file test ---"
tests/test.py << TEST
lfs_format(&lfs, &cfg) => 0;
TEST
tests/test.py << TEST
// Create 300 files of 6 bytes
lfs_mount(&lfs, &cfg) => 0;
lfs_mkdir(&lfs, "directory") => 0;
for (unsigned i = 0; i < 300; i++) {
snprintf((char*)buffer, sizeof(buffer), "file_%03d", i);
lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_WRONLY | LFS_O_CREAT) => 0;
size = 6;
memcpy(wbuffer, "Hello", size);
lfs_file_write(&lfs, &file[0], wbuffer, size) => size;
lfs_file_close(&lfs, &file[0]) => 0;
}
lfs_unmount(&lfs) => 0;
TEST

echo "--- Results ---"
tests/stats.py

0 comments on commit 9ee112a

Please sign in to comment.