Compare commits

...

2 Commits

Author SHA1 Message Date
Christopher Haster
015b86bc51 Fixed issue with trailing dots in file paths
Paths such as the following were causing issues:
/tea/hottea/.
/tea/hottea/..

Unfortunately the existing structure for path lookup didn't make it very
easy to introduce proper handling in this case without duplicating the
entire skip logic for paths. So the lfs_dir_find function had to be
restructured a bit.

One odd side-effect of this is that now lfs_dir_find includes the
initial fetch operation. This kinda breaks the fetch -> op pattern of
the dir functions, but does come with a nice code size reduction.
2018-04-22 07:26:31 -05:00
Christopher Haster
9637b96069 Fixed lookahead overflow and removed unbounded lookahead pointers
As pointed out by davidefer, the lookahead pointer modular arithmetic
does not work around integer overflow when the pointer size is not a
multiple of the block count.

To avoid overflow problems, the easy solution is to stop trying to
work around integer overflows and keep the lookahead offset inside the
block device. To make this work, the ack was modified into a resetable
counter that is decremented every block allocation.

As a plus, quite a bit of the allocation logic ended up simplified.
2018-04-11 14:38:25 -05:00
3 changed files with 70 additions and 93 deletions

143
lfs.c
View File

@@ -270,8 +270,7 @@ int lfs_deorphan(lfs_t *lfs);
static int lfs_alloc_lookahead(void *p, lfs_block_t block) { static int lfs_alloc_lookahead(void *p, lfs_block_t block) {
lfs_t *lfs = p; lfs_t *lfs = p;
lfs_block_t off = (((lfs_soff_t)(block - lfs->free.begin) lfs_block_t off = ((block - lfs->free.off)
% (lfs_soff_t)(lfs->cfg->block_count))
+ lfs->cfg->block_count) % lfs->cfg->block_count; + lfs->cfg->block_count) % lfs->cfg->block_count;
if (off < lfs->free.size) { if (off < lfs->free.size) {
@@ -283,20 +282,22 @@ static int lfs_alloc_lookahead(void *p, lfs_block_t block) {
static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
while (true) { while (true) {
while (lfs->free.off != lfs->free.size) { while (lfs->free.i != lfs->free.size) {
lfs_block_t off = lfs->free.off; lfs_block_t off = lfs->free.i;
lfs->free.off += 1; lfs->free.i += 1;
lfs->free.ack -= 1;
if (!(lfs->free.buffer[off / 32] & (1U << (off % 32)))) { if (!(lfs->free.buffer[off / 32] & (1U << (off % 32)))) {
// found a free block // found a free block
*block = (lfs->free.begin + off) % lfs->cfg->block_count; *block = (lfs->free.off + off) % lfs->cfg->block_count;
// eagerly find next off so an alloc ack can // eagerly find next off so an alloc ack can
// discredit old lookahead blocks // discredit old lookahead blocks
while (lfs->free.off != lfs->free.size && while (lfs->free.i != lfs->free.size &&
(lfs->free.buffer[lfs->free.off / 32] & (lfs->free.buffer[lfs->free.i / 32]
(1U << (lfs->free.off % 32)))) { & (1U << (lfs->free.i % 32)))) {
lfs->free.off += 1; lfs->free.i += 1;
lfs->free.ack -= 1;
} }
return 0; return 0;
@@ -304,15 +305,15 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
} }
// check if we have looked at all blocks since last ack // check if we have looked at all blocks since last ack
if (lfs->free.off == lfs->free.ack - lfs->free.begin) { if (lfs->free.ack == 0) {
LFS_WARN("No more free space %d", lfs->free.off + lfs->free.begin); LFS_WARN("No more free space %d", lfs->free.i + lfs->free.off);
return LFS_ERR_NOSPC; return LFS_ERR_NOSPC;
} }
lfs->free.begin += lfs->free.size; lfs->free.off = (lfs->free.off + lfs->free.size)
lfs->free.size = lfs_min(lfs->cfg->lookahead, % lfs->cfg->block_count;
lfs->free.ack - lfs->free.begin); lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->free.ack);
lfs->free.off = 0; lfs->free.i = 0;
// find mask of free blocks from tree // find mask of free blocks from tree
memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8); memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8);
@@ -324,7 +325,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
} }
static void lfs_alloc_ack(lfs_t *lfs) { static void lfs_alloc_ack(lfs_t *lfs) {
lfs->free.ack = lfs->free.begin+lfs->free.off + lfs->cfg->block_count; lfs->free.ack = lfs->cfg->block_count;
} }
@@ -782,26 +783,19 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir,
lfs_entry_t *entry, const char **path) { lfs_entry_t *entry, const char **path) {
const char *pathname = *path; const char *pathname = *path;
size_t pathlen; size_t pathlen;
entry->d.type = LFS_TYPE_DIR;
entry->d.elen = sizeof(entry->d) - 4;
entry->d.alen = 0;
entry->d.nlen = 0;
entry->d.u.dir[0] = lfs->root[0];
entry->d.u.dir[1] = lfs->root[1];
while (true) { while (true) {
nextname: nextname:
// skip slashes // skip slashes
pathname += strspn(pathname, "/"); pathname += strspn(pathname, "/");
pathlen = strcspn(pathname, "/"); pathlen = strcspn(pathname, "/");
// special case for root dir
if (pathname[0] == '\0') {
*entry = (lfs_entry_t){
.d.type = LFS_TYPE_DIR,
.d.elen = sizeof(entry->d) - 4,
.d.alen = 0,
.d.nlen = 0,
.d.u.dir[0] = lfs->root[0],
.d.u.dir[1] = lfs->root[1],
};
return 0;
}
// skip '.' and root '..' // skip '.' and root '..'
if ((pathlen == 1 && memcmp(pathname, ".", 1) == 0) || if ((pathlen == 1 && memcmp(pathname, ".", 1) == 0) ||
(pathlen == 2 && memcmp(pathname, "..", 2) == 0)) { (pathlen == 2 && memcmp(pathname, "..", 2) == 0)) {
@@ -833,10 +827,25 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir,
suffix += sufflen; suffix += sufflen;
} }
// found path
if (pathname[0] == '\0') {
return 0;
}
// update what we've found // update what we've found
*path = pathname; *path = pathname;
// find path // continue on if we hit a directory
if (entry->d.type != LFS_TYPE_DIR) {
return LFS_ERR_NOTDIR;
}
int err = lfs_dir_fetch(lfs, dir, entry->d.u.dir);
if (err) {
return err;
}
// find entry matching name
while (true) { while (true) {
int err = lfs_dir_next(lfs, dir, entry); int err = lfs_dir_next(lfs, dir, entry);
if (err) { if (err) {
@@ -872,21 +881,8 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir,
entry->d.type &= ~0x80; entry->d.type &= ~0x80;
} }
// to next name
pathname += pathlen; pathname += pathlen;
pathname += strspn(pathname, "/");
if (pathname[0] == '\0') {
return 0;
}
// continue on if we hit a directory
if (entry->d.type != LFS_TYPE_DIR) {
return LFS_ERR_NOTDIR;
}
int err = lfs_dir_fetch(lfs, dir, entry->d.u.dir);
if (err) {
return err;
}
} }
} }
@@ -903,13 +899,8 @@ int lfs_mkdir(lfs_t *lfs, const char *path) {
// fetch parent directory // fetch parent directory
lfs_dir_t cwd; lfs_dir_t cwd;
int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
if (err) {
return err;
}
lfs_entry_t entry; lfs_entry_t entry;
err = lfs_dir_find(lfs, &cwd, &entry, &path); int err = lfs_dir_find(lfs, &cwd, &entry, &path);
if (err != LFS_ERR_NOENT || strchr(path, '/') != NULL) { if (err != LFS_ERR_NOENT || strchr(path, '/') != NULL) {
return err ? err : LFS_ERR_EXIST; return err ? err : LFS_ERR_EXIST;
} }
@@ -953,13 +944,8 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) {
dir->pair[0] = lfs->root[0]; dir->pair[0] = lfs->root[0];
dir->pair[1] = lfs->root[1]; dir->pair[1] = lfs->root[1];
int err = lfs_dir_fetch(lfs, dir, dir->pair);
if (err) {
return err;
}
lfs_entry_t entry; lfs_entry_t entry;
err = lfs_dir_find(lfs, dir, &entry, &path); int err = lfs_dir_find(lfs, dir, &entry, &path);
if (err) { if (err) {
return err; return err;
} else if (entry.d.type != LFS_TYPE_DIR) { } else if (entry.d.type != LFS_TYPE_DIR) {
@@ -1301,13 +1287,8 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file,
// allocate entry for file if it doesn't exist // allocate entry for file if it doesn't exist
lfs_dir_t cwd; lfs_dir_t cwd;
int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
if (err) {
return err;
}
lfs_entry_t entry; lfs_entry_t entry;
err = lfs_dir_find(lfs, &cwd, &entry, &path); int err = lfs_dir_find(lfs, &cwd, &entry, &path);
if (err && (err != LFS_ERR_NOENT || strchr(path, '/') != NULL)) { if (err && (err != LFS_ERR_NOENT || strchr(path, '/') != NULL)) {
return err; return err;
} }
@@ -1813,13 +1794,8 @@ lfs_soff_t lfs_file_size(lfs_t *lfs, lfs_file_t *file) {
/// General fs operations /// /// General fs operations ///
int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info) { int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info) {
lfs_dir_t cwd; lfs_dir_t cwd;
int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
if (err) {
return err;
}
lfs_entry_t entry; lfs_entry_t entry;
err = lfs_dir_find(lfs, &cwd, &entry, &path); int err = lfs_dir_find(lfs, &cwd, &entry, &path);
if (err) { if (err) {
return err; return err;
} }
@@ -1854,13 +1830,8 @@ int lfs_remove(lfs_t *lfs, const char *path) {
} }
lfs_dir_t cwd; lfs_dir_t cwd;
int err = lfs_dir_fetch(lfs, &cwd, lfs->root);
if (err) {
return err;
}
lfs_entry_t entry; lfs_entry_t entry;
err = lfs_dir_find(lfs, &cwd, &entry, &path); int err = lfs_dir_find(lfs, &cwd, &entry, &path);
if (err) { if (err) {
return err; return err;
} }
@@ -1915,24 +1886,14 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
// find old entry // find old entry
lfs_dir_t oldcwd; lfs_dir_t oldcwd;
int err = lfs_dir_fetch(lfs, &oldcwd, lfs->root);
if (err) {
return err;
}
lfs_entry_t oldentry; lfs_entry_t oldentry;
err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath); int err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath);
if (err) { if (err) {
return err; return err;
} }
// allocate new entry // allocate new entry
lfs_dir_t newcwd; lfs_dir_t newcwd;
err = lfs_dir_fetch(lfs, &newcwd, lfs->root);
if (err) {
return err;
}
lfs_entry_t preventry; lfs_entry_t preventry;
err = lfs_dir_find(lfs, &newcwd, &preventry, &newpath); err = lfs_dir_find(lfs, &newcwd, &preventry, &newpath);
if (err && (err != LFS_ERR_NOENT || strchr(newpath, '/') != NULL)) { if (err && (err != LFS_ERR_NOENT || strchr(newpath, '/') != NULL)) {
@@ -2103,9 +2064,9 @@ int lfs_format(lfs_t *lfs, const struct lfs_config *cfg) {
// create free lookahead // create free lookahead
memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8); memset(lfs->free.buffer, 0, lfs->cfg->lookahead/8);
lfs->free.begin = 0;
lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count);
lfs->free.off = 0; lfs->free.off = 0;
lfs->free.size = lfs_min(lfs->cfg->lookahead, lfs->cfg->block_count);
lfs->free.i = 0;
lfs_alloc_ack(lfs); lfs_alloc_ack(lfs);
// create superblock dir // create superblock dir
@@ -2182,9 +2143,9 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) {
} }
// setup free lookahead // setup free lookahead
lfs->free.begin = 0;
lfs->free.size = 0;
lfs->free.off = 0; lfs->free.off = 0;
lfs->free.size = 0;
lfs->free.i = 0;
lfs_alloc_ack(lfs); lfs_alloc_ack(lfs);
// load superblock // load superblock

4
lfs.h
View File

@@ -259,9 +259,9 @@ typedef struct lfs_superblock {
} lfs_superblock_t; } lfs_superblock_t;
typedef struct lfs_free { typedef struct lfs_free {
lfs_block_t begin;
lfs_block_t size;
lfs_block_t off; lfs_block_t off;
lfs_block_t size;
lfs_block_t i;
lfs_block_t ack; lfs_block_t ack;
uint32_t *buffer; uint32_t *buffer;
} lfs_free_t; } lfs_free_t;

View File

@@ -90,6 +90,22 @@ tests/test.py << TEST
lfs_unmount(&lfs) => 0; lfs_unmount(&lfs) => 0;
TEST TEST
echo "--- Trailing dot path tests ---"
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_stat(&lfs, "tea/hottea/", &info) => 0;
strcmp(info.name, "hottea") => 0;
lfs_stat(&lfs, "tea/hottea/.", &info) => 0;
strcmp(info.name, "hottea") => 0;
lfs_stat(&lfs, "tea/hottea/./.", &info) => 0;
strcmp(info.name, "hottea") => 0;
lfs_stat(&lfs, "tea/hottea/..", &info) => 0;
strcmp(info.name, "tea") => 0;
lfs_stat(&lfs, "tea/hottea/../.", &info) => 0;
strcmp(info.name, "tea") => 0;
lfs_unmount(&lfs) => 0;
TEST
echo "--- Root dot dot path tests ---" echo "--- Root dot dot path tests ---"
tests/test.py << TEST tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0; lfs_mount(&lfs, &cfg) => 0;