Skip to content

Commit

Permalink
paths: Fixed/doc trailing slash/dot POSIX incompatibilities
Browse files Browse the repository at this point in the history
- lfs_mkdir now accepts trailing slashes:
  - before: lfs_mkdir("a/") => LFS_ERR_NOENT
  - after:  lfs_mkdir("a/") => 0

- lfs_stat, lfs_getattr, etc, now reject trailing slashes if the file is
  not a directory:
  - before: lfs_stat("reg_a/") => 0
  - after:  lfs_stat("reg_a/") => LFS_ERR_NOTDIR

  Note trailing slashes are accepted if the file is a directory:
  - before: lfs_stat("dir_a/") => 0
  - after:  lfs_stat("dir_a/") => 0

- lfs_file_open now returns LFS_ERR_NOTDIR if the file exists but the
  path contains trailing slashes:
  - before: lfs_file_open("reg_a/") => LFS_ERR_NOENT
  - after:  lfs_file_open("reg_a/") => LFS_ERR_NOTDIR

To make these work, the internal lfs_dir_find API required some
interesting changes:

- lfs_dir_find no longer sets id=0x3ff on not finding a parent entry in
  the path. Instead, lfs_path_islast can be used to determine if the
  modified path references a parent entry or child entry based on the
  remainder of the path string.

  Note this is only necessary for functions that create new entries
  (lfs_mkdir, lfs_rename, lfs_file_open).

- Trailing slashes mean we can no longer rely on the modified path being
  NULL-terminated. lfs_path_namelen provides an alternative to strlen
  that stops at slash or NULL.

- lfs_path_isdir also tells you if the modified path must reference a
  dir (contains trailing slashes). I considered handling this entirely
  in lfs_dir_find, but the behavior of entry-creating functions is too
  nuanced.

  At least lfs_dir_find returns LFS_ERR_NOTDIR if the file exists on
  disk.

Like strlen, lfs_path_namelen/islast/isdir are all O(n) where n is the
name length. This isn't great, but if you're using filenames large
enough for this to actually matter... uh... open an issue on GitHub and
we might improve this in the future.

---

There are a couple POSIX incompatibilities that I think are not
worth fixing:

- Root modifications return EINVAL instead of EBUSY:
  - littlefs: remove("/") => EINVAL
  - POSIX:    remove("/") => EBUSY
  Reason: This would be the only use of EBUSY in the system.

- We accept modifications of directories with trailing dots:
  - littlefs: remove("a/.") => 0
  - POSIX:    remove("a/.") => EBUSY
  Reason: Not worth implementing.

- We do not check for existence of directories followed by dotdots:
  - littlefs: stat("a/missing/..") => 0
  - POSIX:    stat("a/missing/..") => ENOENT
  Reason: Difficult to implement non-recursively.

- We accept modifications of directories with trailing dotdots:
  - littlefs: rename("a/b/..", "c") => 0
  - POSIX:    rename("a/b/..", "c") => EBUSY
  Reason: Not worth implementing.

These are at least now documented in tests/test_paths.toml, which isn't
the greatest location, but it's at least something until a better
document is created.

Note that these don't really belong in SPEC.md because path parsing is
a function of the driver and has no impact on disk.
  • Loading branch information
geky committed Nov 25, 2024
1 parent 232e736 commit a603507
Show file tree
Hide file tree
Showing 2 changed files with 299 additions and 291 deletions.
68 changes: 53 additions & 15 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,21 @@ static int lfs_bd_erase(lfs_t *lfs, lfs_block_t block) {


/// Small type-level utilities ///

// some operations on paths
static inline lfs_size_t lfs_path_namelen(const char *path) {
return strcspn(path, "/");
}

static inline bool lfs_path_islast(const char *path) {
lfs_size_t namelen = lfs_path_namelen(path);
return path[namelen + strspn(path + namelen, "/")] == '\0';
}

static inline bool lfs_path_isdir(const char *path) {
return path[lfs_path_namelen(path)] != '\0';
}

// operations on block pairs
static inline void lfs_pair_swap(lfs_block_t pair[2]) {
lfs_block_t t = pair[0];
Expand Down Expand Up @@ -1461,13 +1476,16 @@ static int lfs_dir_find_match(void *data,
return LFS_CMP_EQ;
}

// lfs_dir_find tries to set path and id even if file is not found
//
// returns:
// - 0 if file is found
// - LFS_ERR_NOENT if file or parent is not found
// - LFS_ERR_NOTDIR if parent is not a dir
static lfs_stag_t lfs_dir_find(lfs_t *lfs, lfs_mdir_t *dir,
const char **path, uint16_t *id) {
// we reduce path to a single name if we can find it
const char *name = *path;
if (id) {
*id = 0x3ff;
}

// default to root dir
lfs_stag_t tag = LFS_MKTAG(LFS_TYPE_DIR, 0x3ff, 0);
Expand All @@ -1476,8 +1494,10 @@ static lfs_stag_t lfs_dir_find(lfs_t *lfs, lfs_mdir_t *dir,

while (true) {
nextname:
// skip slashes
name += strspn(name, "/");
// skip slashes if we're a directory
if (lfs_tag_type3(tag) == LFS_TYPE_DIR) {
name += strspn(name, "/");
}
lfs_size_t namelen = strcspn(name, "/");

// skip '.' and root '..'
Expand Down Expand Up @@ -1519,7 +1539,7 @@ static lfs_stag_t lfs_dir_find(lfs_t *lfs, lfs_mdir_t *dir,
// update what we've found so far
*path = name;

// only continue if we hit a directory
// only continue if we're a directory
if (lfs_tag_type3(tag) != LFS_TYPE_DIR) {
return LFS_ERR_NOTDIR;
}
Expand All @@ -1539,8 +1559,7 @@ static lfs_stag_t lfs_dir_find(lfs_t *lfs, lfs_mdir_t *dir,
tag = lfs_dir_fetchmatch(lfs, dir, dir->tail,
LFS_MKTAG(0x780, 0, 0),
LFS_MKTAG(LFS_TYPE_NAME, 0, namelen),
// are we last name?
(strchr(name, '/') == NULL) ? id : NULL,
id,
lfs_dir_find_match, &(struct lfs_dir_find_match){
lfs, name, namelen});
if (tag < 0) {
Expand Down Expand Up @@ -2603,12 +2622,12 @@ static int lfs_mkdir_(lfs_t *lfs, const char *path) {
cwd.next = lfs->mlist;
uint16_t id;
err = lfs_dir_find(lfs, &cwd.m, &path, &id);
if (!(err == LFS_ERR_NOENT && id != 0x3ff)) {
if (!(err == LFS_ERR_NOENT && lfs_path_islast(path))) {
return (err < 0) ? err : LFS_ERR_EXIST;
}

// check that name fits
lfs_size_t nlen = strlen(path);
lfs_size_t nlen = lfs_path_namelen(path);
if (nlen > lfs->name_max) {
return LFS_ERR_NAMETOOLONG;
}
Expand Down Expand Up @@ -3057,7 +3076,7 @@ static int lfs_file_opencfg_(lfs_t *lfs, lfs_file_t *file,

// allocate entry for file if it doesn't exist
lfs_stag_t tag = lfs_dir_find(lfs, &file->m, &path, &file->id);
if (tag < 0 && !(tag == LFS_ERR_NOENT && file->id != 0x3ff)) {
if (tag < 0 && !(tag == LFS_ERR_NOENT && lfs_path_islast(path))) {
err = tag;
goto cleanup;
}
Expand All @@ -3077,8 +3096,14 @@ static int lfs_file_opencfg_(lfs_t *lfs, lfs_file_t *file,
goto cleanup;
}

// don't allow trailing slashes
if (lfs_path_isdir(path)) {
err = LFS_ERR_ISDIR;
goto cleanup;
}

// check that name fits
lfs_size_t nlen = strlen(path);
lfs_size_t nlen = lfs_path_namelen(path);
if (nlen > lfs->name_max) {
err = LFS_ERR_NAMETOOLONG;
goto cleanup;
Expand Down Expand Up @@ -3842,6 +3867,12 @@ static int lfs_stat_(lfs_t *lfs, const char *path, struct lfs_info *info) {
return (int)tag;
}

// only allow trailing slashes on dirs
if (strchr(path, '/') != NULL
&& lfs_tag_type3(tag) != LFS_TYPE_DIR) {
return LFS_ERR_NOTDIR;
}

return lfs_dir_getinfo(lfs, &cwd, lfs_tag_id(tag), info);
}

Expand Down Expand Up @@ -3944,7 +3975,7 @@ static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) {
uint16_t newid;
lfs_stag_t prevtag = lfs_dir_find(lfs, &newcwd, &newpath, &newid);
if ((prevtag < 0 || lfs_tag_id(prevtag) == 0x3ff) &&
!(prevtag == LFS_ERR_NOENT && newid != 0x3ff)) {
!(prevtag == LFS_ERR_NOENT && lfs_path_islast(newpath))) {
return (prevtag < 0) ? (int)prevtag : LFS_ERR_INVAL;
}

Expand All @@ -3955,8 +3986,14 @@ static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) {
struct lfs_mlist prevdir;
prevdir.next = lfs->mlist;
if (prevtag == LFS_ERR_NOENT) {
// if we're a file, don't allow trailing slashes
if (lfs_path_isdir(newpath)
&& lfs_tag_type3(oldtag) != LFS_TYPE_DIR) {
return LFS_ERR_NOTDIR;
}

// check that name fits
lfs_size_t nlen = strlen(newpath);
lfs_size_t nlen = lfs_path_namelen(newpath);
if (nlen > lfs->name_max) {
return LFS_ERR_NAMETOOLONG;
}
Expand Down Expand Up @@ -4016,7 +4053,8 @@ static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) {
{LFS_MKTAG_IF(prevtag != LFS_ERR_NOENT,
LFS_TYPE_DELETE, newid, 0), NULL},
{LFS_MKTAG(LFS_TYPE_CREATE, newid, 0), NULL},
{LFS_MKTAG(lfs_tag_type3(oldtag), newid, strlen(newpath)), newpath},
{LFS_MKTAG(lfs_tag_type3(oldtag),
newid, lfs_path_namelen(newpath)), newpath},
{LFS_MKTAG(LFS_FROM_MOVE, newid, lfs_tag_id(oldtag)), &oldcwd},
{LFS_MKTAG_IF(samepair,
LFS_TYPE_DELETE, newoldid, 0), NULL}));
Expand Down
Loading

0 comments on commit a603507

Please sign in to comment.