From 999ef6656f13eeaa0de7f7d880ba6d1937aa60dd Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 25 Nov 2024 15:22:33 -0600 Subject: [PATCH] paths: Changed CREAT with a trailing slash to return NOTDIR - before: lfs_file_open("missing/") => LFS_ERR_ISDIR - after: lfs_file_open("missing/") => 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. --- lfs.c | 2 +- tests/test_paths.toml | 36 ++++++++++++++++++------------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lfs.c b/lfs.c index 94d23f7b..b46cca5b 100644 --- a/lfs.c +++ b/lfs.c @@ -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; } diff --git a/tests/test_paths.toml b/tests/test_paths.toml index d8ae57f9..29574fa1 100644 --- a/tests/test_paths.toml +++ b/tests/test_paths.toml @@ -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", @@ -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;