Skip to content

Commit

Permalink
paths: Changed CREAT with a trailing slash to return NOTDIR
Browse files Browse the repository at this point in the history
- before: lfs_file_open("missing_a/") => LFS_ERR_ISDIR
- after:  lfs_file_open("missing_a/") => LFS_ERR_NOTDIR

As noted by bmcdonnell-fb, returning LFS_ERR_ISDIR here was inconsistent
with the case where the file exists:

  case                           before          after
  lfs_file_open("dir_a")      => LFS_ERR_ISDIR   LFS_ERR_ISDIR
  lfs_file_open("dir_a/")     => LFS_ERR_ISDIR   LFS_ERR_ISDIR
  lfs_file_open("reg_a/")     => LFS_ERR_NOTDIR  LFS_ERR_NOTDIR
  lfs_file_open("missing_a/") => LFS_ERR_ISDIR   LFS_ERR_NOTDIR

Note this is consistent with the behavior of lfs_stat:

  lfs_file_open("reg_a/") => LFS_ERR_NOTDIR
  lfs_stat("reg_a/")      => LFS_ERR_NOTDIR

And the only other function that can "create" files, lfs_rename:

  lfs_file_open("missing_a/")       => LFS_ERR_NOTDIR
  lfs_rename("reg_a", "missing_a/") => LFS_ERR_NOTDIR

There is some ongoing discussion about if these should return NOTDIR,
ISDIR, or INVAL, but this is at least an improvement over the
rename/open mismatch.
  • Loading branch information
geky committed Nov 25, 2024
1 parent f9e0c1d commit 21de7dc
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 19 deletions.
2 changes: 1 addition & 1 deletion lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -3109,7 +3109,7 @@ static int lfs_file_opencfg_(lfs_t *lfs, lfs_file_t *file,

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

Expand Down
36 changes: 18 additions & 18 deletions tests/test_paths.toml
Original file line number Diff line number Diff line change
Expand Up @@ -764,17 +764,17 @@ code = '''
} else {
lfs_file_t file;
lfs_file_open(&lfs, &file, "coffee/drip/",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/coldbrew//",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/turkish///",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/tubruk////",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/vietnamese/////",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/thai//////",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;

// still create so we have something to test
lfs_file_open(&lfs, &file, "coffee/drip",
Expand Down Expand Up @@ -3972,30 +3972,30 @@ code = '''
LFS_O_RDONLY) => LFS_ERR_NOENT;

lfs_file_open(&lfs, &file, "coffee/_rip/",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/c_ldbrew//",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/tu_kish///",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/tub_uk////",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/_vietnamese/////",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/thai_//////",
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR;

lfs_file_open(&lfs, &file, "coffee/_rip/",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/c_ldbrew//",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/tu_kish///",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/tub_uk////",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/_vietnamese/////",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;
lfs_file_open(&lfs, &file, "coffee/thai_//////",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_ISDIR;
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_NOTDIR;

// dir open paths, only works on dirs!
lfs_dir_t dir;
Expand Down

0 comments on commit 21de7dc

Please sign in to comment.