From 499a6339764b51f2ebde2081c33fc1936be9b8dd Mon Sep 17 00:00:00 2001 From: Chris Friedt Date: Sat, 15 Jun 2024 12:13:56 -0400 Subject: [PATCH] posix: device_io: use mode argument correctly in open() Previously, we had only used the flags field and ignored mode with the open() function. Signed-off-by: Chris Friedt --- 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, 67 insertions(+), 50 deletions(-) diff --git a/include/zephyr/posix/fcntl.h b/include/zephyr/posix/fcntl.h index 9689d1ae8c3f..9aeef0caa6fb 100644 --- a/include/zephyr/posix/fcntl.h +++ b/include/zephyr/posix/fcntl.h @@ -8,9 +8,13 @@ #define ZEPHYR_POSIX_FCNTL_H_ #ifdef CONFIG_PICOLIBC -#define O_CREAT 0x0040 +#define O_CREAT 0x0040 +#define O_TRUNC 0x0200 +#define O_APPEND 0x0400 #else -#define O_CREAT 0x0200 +#define O_APPEND 0x0008 +#define O_CREAT 0x0200 +#define O_TRUNC 0x0400 #endif #define O_ACCMODE (O_RDONLY | O_WRONLY | O_RDWR) @@ -19,7 +23,6 @@ #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 a504545ba034..6fea22d01545 100644 --- a/lib/posix/options/device_io.c +++ b/lib/posix/options/device_io.c @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -16,28 +17,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 zvfs_open(const char *name, int flags, int mode); 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, (struct zvfs_fd_set *)fdset); + return ZVFS_FD_CLR(fd, fdset); } int FD_ISSET(int fd, struct zvfs_fd_set *fdset) { - return ZVFS_FD_ISSET(fd, (struct zvfs_fd_set *)fdset); + return ZVFS_FD_ISSET(fd, fdset); } void FD_SET(int fd, struct zvfs_fd_set *fdset) { - ZVFS_FD_SET(fd, (struct zvfs_fd_set *)fdset); + ZVFS_FD_SET(fd, fdset); } void FD_ZERO(fd_set *fdset) { - ZVFS_FD_ZERO((struct zvfs_fd_set *)fdset); + ZVFS_FD_ZERO(fdset); } int close(int fd) @@ -60,8 +61,16 @@ int fileno(FILE *file) int open(const char *name, int flags, ...) { - /* FIXME: necessarily need to check for O_CREAT and unpack ... if set */ - return zvfs_open(name, 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); } #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 a8bf1fb620fc..a9d07b73293b 100644 --- a/lib/posix/options/fs.c +++ b/lib/posix/options/fs.c @@ -59,37 +59,22 @@ static inline void posix_fs_free_obj(struct posix_fs_desc *ptr) ptr->used = false; } -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 zvfs_open(const char *name, int flags, int mode) { int rc, fd; struct posix_fs_desc *ptr = NULL; - int zmode = posix_mode_to_zephyr(flags); + int zflags = 0; + + 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 (zmode < 0) { - return zmode; + if ((flags & O_APPEND) != 0) { + zflags |= FS_O_APPEND; } fd = zvfs_reserve_fd(); @@ -99,24 +84,44 @@ int zvfs_open(const char *name, int flags) ptr = posix_fs_alloc_obj(false); if (ptr == NULL) { - zvfs_free_fd(fd); - errno = EMFILE; - return -1; + rc = -EMFILE; + goto out_err; } fs_file_t_init(&ptr->file); - rc = fs_open(&ptr->file, name, zmode); + if (flags & O_CREAT) { + flags &= ~O_CREAT; + 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) { - posix_fs_free_obj(ptr); - zvfs_free_fd(fd); - errno = -rc; - return -1; + goto out_err; } 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 b0908cb1586c..dd3a89c6dc57 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); + res = open(TEST_DIR_FILE, O_CREAT | O_RDWR, 0770); 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 9614412a07a5..c6f2edd66895 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); + res = open(TEST_FILE, O_CREAT | O_RDWR, 0660); 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 1187b1c3e575..7c35145343b7 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); + fd = open(THE_FILE, O_CREAT | O_WRONLY, 0440); 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); + fd = open(THE_FILE, O_CREAT | O_WRONLY, 0440); 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 14167d3f7756..0031596dcef1 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); + fh = open(filename, O_CREAT | O_WRONLY, 0440); zassert(fh >= 0, "Failed creating test file"); uint8_t filling[FILL_SIZE];