From ade54b35cae9b4cb987f4bcef12702d7ddaf3587 Mon Sep 17 00:00:00 2001 From: Alberto Escolar Piedras Date: Mon, 1 Jul 2024 12:07:18 +0200 Subject: [PATCH] Revert "posix: device_io: use mode argument correctly in open()" This reverts commit 499a6339764b51f2ebde2081c33fc1936be9b8dd. PR #73978 introduced a regression. Unfortunately this PR cannot be reverted without reverting also Let's revert both PRs to stabilize main again towards the 3.7 release. For more details on the issue see https://github.com/zephyrproject-rtos/zephyr/issues/75205 Signed-off-by: Alberto Escolar Piedras --- include/zephyr/posix/fcntl.h | 9 +-- lib/posix/options/device_io.c | 23 +++----- lib/posix/options/fs.c | 75 ++++++++++++------------- tests/posix/fs/src/test_fs_dir.c | 2 +- tests/posix/fs/src/test_fs_file.c | 2 +- tests/posix/fs/src/test_fs_open_flags.c | 4 +- tests/posix/fs/src/test_fs_stat.c | 2 +- 7 files changed, 50 insertions(+), 67 deletions(-) diff --git a/include/zephyr/posix/fcntl.h b/include/zephyr/posix/fcntl.h index 9aeef0caa6f..9689d1ae8c3 100644 --- a/include/zephyr/posix/fcntl.h +++ b/include/zephyr/posix/fcntl.h @@ -8,13 +8,9 @@ #define ZEPHYR_POSIX_FCNTL_H_ #ifdef CONFIG_PICOLIBC -#define O_CREAT 0x0040 -#define O_TRUNC 0x0200 -#define O_APPEND 0x0400 +#define O_CREAT 0x0040 #else -#define O_APPEND 0x0008 -#define O_CREAT 0x0200 -#define O_TRUNC 0x0400 +#define O_CREAT 0x0200 #endif #define O_ACCMODE (O_RDONLY | O_WRONLY | O_RDWR) @@ -23,6 +19,7 @@ #define O_WRONLY 01 #define O_RDWR 02 +#define O_APPEND 0x0400 #define O_EXCL 0x0800 #define O_NONBLOCK 0x4000 diff --git a/lib/posix/options/device_io.c b/lib/posix/options/device_io.c index 6fea22d0154..a504545ba03 100644 --- a/lib/posix/options/device_io.c +++ b/lib/posix/options/device_io.c @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -17,28 +16,28 @@ int zvfs_close(int fd); FILE *zvfs_fdopen(int fd, const char *mode); int zvfs_fileno(FILE *file); -int zvfs_open(const char *name, int flags, int mode); +int zvfs_open(const char *name, int flags); ssize_t zvfs_read(int fd, void *buf, size_t sz, size_t *from_offset); ssize_t zvfs_write(int fd, const void *buf, size_t sz, size_t *from_offset); void FD_CLR(int fd, struct zvfs_fd_set *fdset) { - return ZVFS_FD_CLR(fd, fdset); + return ZVFS_FD_CLR(fd, (struct zvfs_fd_set *)fdset); } int FD_ISSET(int fd, struct zvfs_fd_set *fdset) { - return ZVFS_FD_ISSET(fd, fdset); + return ZVFS_FD_ISSET(fd, (struct zvfs_fd_set *)fdset); } void FD_SET(int fd, struct zvfs_fd_set *fdset) { - ZVFS_FD_SET(fd, fdset); + ZVFS_FD_SET(fd, (struct zvfs_fd_set *)fdset); } void FD_ZERO(fd_set *fdset) { - ZVFS_FD_ZERO(fdset); + ZVFS_FD_ZERO((struct zvfs_fd_set *)fdset); } int close(int fd) @@ -61,16 +60,8 @@ int fileno(FILE *file) int open(const char *name, int flags, ...) { - int mode = 0; - va_list args; - - if ((flags & O_CREAT) != 0) { - va_start(args, flags); - mode = va_arg(args, int); - va_end(args); - } - - return zvfs_open(name, flags, mode); + /* FIXME: necessarily need to check for O_CREAT and unpack ... if set */ + return zvfs_open(name, flags); } #ifdef CONFIG_POSIX_DEVICE_IO_ALIAS_OPEN FUNC_ALIAS(open, _open, int); diff --git a/lib/posix/options/fs.c b/lib/posix/options/fs.c index a9d07b73293..a8bf1fb620f 100644 --- a/lib/posix/options/fs.c +++ b/lib/posix/options/fs.c @@ -59,22 +59,37 @@ static inline void posix_fs_free_obj(struct posix_fs_desc *ptr) ptr->used = false; } -int zvfs_open(const char *name, int flags, int mode) +static int posix_mode_to_zephyr(int mf) +{ + int mode = (mf & O_CREAT) ? FS_O_CREATE : 0; + + mode |= (mf & O_APPEND) ? FS_O_APPEND : 0; + + switch (mf & O_ACCMODE) { + case O_RDONLY: + mode |= FS_O_READ; + break; + case O_WRONLY: + mode |= FS_O_WRITE; + break; + case O_RDWR: + mode |= FS_O_RDWR; + break; + default: + break; + } + + return mode; +} + +int zvfs_open(const char *name, int flags) { int rc, fd; struct posix_fs_desc *ptr = NULL; - int zflags = 0; + int zmode = posix_mode_to_zephyr(flags); - if ((flags & O_ACCMODE) == O_RDONLY) { - zflags |= FS_O_READ; - } else if ((flags & O_ACCMODE) == O_WRONLY) { - zflags |= FS_O_WRITE; - } else if ((flags & O_ACCMODE) == O_RDWR) { - zflags |= FS_O_RDWR; - } - - if ((flags & O_APPEND) != 0) { - zflags |= FS_O_APPEND; + if (zmode < 0) { + return zmode; } fd = zvfs_reserve_fd(); @@ -84,44 +99,24 @@ int zvfs_open(const char *name, int flags, int mode) ptr = posix_fs_alloc_obj(false); if (ptr == NULL) { - rc = -EMFILE; - goto out_err; + zvfs_free_fd(fd); + errno = EMFILE; + return -1; } fs_file_t_init(&ptr->file); - if (flags & O_CREAT) { - flags &= ~O_CREAT; + rc = fs_open(&ptr->file, name, zmode); - rc = fs_open(&ptr->file, name, FS_O_CREATE | (mode & O_ACCMODE)); - if (rc < 0) { - goto out_err; - } - rc = fs_close(&ptr->file); - if (rc < 0) { - goto out_err; - } - } - - rc = fs_open(&ptr->file, name, zflags); if (rc < 0) { - goto out_err; + posix_fs_free_obj(ptr); + zvfs_free_fd(fd); + errno = -rc; + return -1; } zvfs_finalize_fd(fd, ptr, &fs_fd_op_vtable); - goto out; - -out_err: - if (ptr != NULL) { - posix_fs_free_obj(ptr); - } - - zvfs_free_fd(fd); - errno = -rc; - return -1; - -out: return fd; } diff --git a/tests/posix/fs/src/test_fs_dir.c b/tests/posix/fs/src/test_fs_dir.c index dd3a89c6dc5..b0908cb1586 100644 --- a/tests/posix/fs/src/test_fs_dir.c +++ b/tests/posix/fs/src/test_fs_dir.c @@ -27,7 +27,7 @@ static int test_mkdir(void) return res; } - res = open(TEST_DIR_FILE, O_CREAT | O_RDWR, 0770); + res = open(TEST_DIR_FILE, O_CREAT | O_RDWR); if (res < 0) { TC_PRINT("Failed opening file [%d]\n", res); diff --git a/tests/posix/fs/src/test_fs_file.c b/tests/posix/fs/src/test_fs_file.c index c6f2edd6689..9614412a07a 100644 --- a/tests/posix/fs/src/test_fs_file.c +++ b/tests/posix/fs/src/test_fs_file.c @@ -16,7 +16,7 @@ static int test_file_open(void) { int res; - res = open(TEST_FILE, O_CREAT | O_RDWR, 0660); + res = open(TEST_FILE, O_CREAT | O_RDWR); if (res < 0) { TC_ERROR("Failed opening file: %d, errno=%d\n", res, errno); /* FIXME: restructure tests as per #46897 */ diff --git a/tests/posix/fs/src/test_fs_open_flags.c b/tests/posix/fs/src/test_fs_open_flags.c index 7c35145343b..1187b1c3e57 100644 --- a/tests/posix/fs/src/test_fs_open_flags.c +++ b/tests/posix/fs/src/test_fs_open_flags.c @@ -60,7 +60,7 @@ static int test_file_open_flags(void) /* 2 Create file for read only, attempt to read, attempt to write */ TC_PRINT("Open on non-existent file, flags = O_CREAT | O_WRONLY\n"); - fd = open(THE_FILE, O_CREAT | O_WRONLY, 0440); + fd = open(THE_FILE, O_CREAT | O_WRONLY); if (fd < 0) { TC_PRINT("Expected success; fd = %d, errno = %d\n", fd, errno); return TC_FAIL; @@ -236,7 +236,7 @@ static int test_file_open_flags(void) TC_PRINT("Attempt write to file opened with O_APPEND | O_RDWR\n"); /* Clean start */ unlink(THE_FILE); - fd = open(THE_FILE, O_CREAT | O_WRONLY, 0440); + fd = open(THE_FILE, O_CREAT | O_WRONLY); if (fd < 0) { TC_PRINT("Expected success, fd = %d, errno = %d\n", fd, errno); return TC_FAIL; diff --git a/tests/posix/fs/src/test_fs_stat.c b/tests/posix/fs/src/test_fs_stat.c index 0031596dcef..14167d3f775 100644 --- a/tests/posix/fs/src/test_fs_stat.c +++ b/tests/posix/fs/src/test_fs_stat.c @@ -21,7 +21,7 @@ static void create_file(const char *filename, uint32_t size) { int fh; - fh = open(filename, O_CREAT | O_WRONLY, 0440); + fh = open(filename, O_CREAT | O_WRONLY); zassert(fh >= 0, "Failed creating test file"); uint8_t filling[FILL_SIZE];