From a4ac3cc6e8de8902e7b0bc6bdddfd3fb54ffeb7a Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Mon, 12 Aug 2024 18:51:27 +0200 Subject: [PATCH] fix `os.read_dir` closing the given file descriptor --- core/os/dir_unix.odin | 6 ++++-- core/os/os_darwin.odin | 12 +++++++++++- core/os/os_freebsd.odin | 10 ++++++++++ core/os/os_linux.odin | 6 ++++++ core/os/os_netbsd.odin | 10 ++++++++++ core/os/os_openbsd.odin | 10 ++++++++++ tests/core/os/os.odin | 6 ++++-- 7 files changed, 55 insertions(+), 5 deletions(-) diff --git a/core/os/dir_unix.odin b/core/os/dir_unix.odin index 6f6bed36d..b472e89b7 100644 --- a/core/os/dir_unix.odin +++ b/core/os/dir_unix.odin @@ -5,10 +5,12 @@ import "core:strings" @(require_results) read_dir :: proc(fd: Handle, n: int, allocator := context.allocator) -> (fi: []File_Info, err: Error) { - dirp := _fdopendir(fd) or_return + dupfd := _dup(fd) or_return + + dirp := _fdopendir(dupfd) or_return defer _closedir(dirp) - dirpath := absolute_path_from_handle(fd) or_return + dirpath := absolute_path_from_handle(dupfd) or_return defer delete(dirpath) n := n diff --git a/core/os/os_darwin.odin b/core/os/os_darwin.odin index f21dbb676..0644ca645 100644 --- a/core/os/os_darwin.odin +++ b/core/os/os_darwin.odin @@ -598,7 +598,8 @@ foreign libc { @(link_name="fstat64") _unix_fstat :: proc(fd: Handle, stat: ^OS_Stat) -> c.int --- @(link_name="readlink") _unix_readlink :: proc(path: cstring, buf: ^byte, bufsiz: c.size_t) -> c.ssize_t --- @(link_name="access") _unix_access :: proc(path: cstring, mask: c.int) -> c.int --- - @(link_name="fsync") _unix_fsync :: proc(handle: Handle) -> c.int --- + @(link_name="fsync") _unix_fsync :: proc(handle: Handle) -> c.int --- + @(link_name="dup") _unix_dup :: proc(handle: Handle) -> Handle --- @(link_name="fdopendir$INODE64") _unix_fdopendir_amd64 :: proc(fd: Handle) -> Dir --- @(link_name="readdir_r$INODE64") _unix_readdir_r_amd64 :: proc(dirp: Dir, entry: ^Dirent, result: ^^Dirent) -> c.int --- @@ -1009,6 +1010,15 @@ _readlink :: proc(path: string) -> (string, Error) { } } +@(private, require_results) +_dup :: proc(fd: Handle) -> (Handle, Error) { + dup := _unix_dup(fd) + if dup == -1 { + return INVALID_HANDLE, get_last_error() + } + return dup, nil +} + @(require_results) absolute_path_from_handle :: proc(fd: Handle) -> (path: string, err: Error) { buf: [DARWIN_MAXPATHLEN]byte diff --git a/core/os/os_freebsd.odin b/core/os/os_freebsd.odin index c7955368e..e37f9767b 100644 --- a/core/os/os_freebsd.odin +++ b/core/os/os_freebsd.odin @@ -389,6 +389,7 @@ foreign libc { @(link_name="rmdir") _unix_rmdir :: proc(path: cstring) -> c.int --- @(link_name="mkdir") _unix_mkdir :: proc(path: cstring, mode: mode_t) -> c.int --- @(link_name="fcntl") _unix_fcntl :: proc(fd: Handle, cmd: c.int, arg: uintptr) -> c.int --- + @(link_name="dup") _unix_dup :: proc(fd: Handle) -> Handle --- @(link_name="fdopendir") _unix_fdopendir :: proc(fd: Handle) -> Dir --- @(link_name="closedir") _unix_closedir :: proc(dirp: Dir) -> c.int --- @@ -739,6 +740,15 @@ _readlink :: proc(path: string) -> (string, Error) { return "", Error{} } +@(private, require_results) +_dup :: proc(fd: Handle) -> (Handle, Error) { + dup := _unix_dup(fd) + if dup == -1 { + return INVALID_HANDLE, get_last_error() + } + return dup, nil +} + @(require_results) absolute_path_from_handle :: proc(fd: Handle) -> (string, Error) { // NOTE(Feoramund): The situation isn't ideal, but this was the best way I diff --git a/core/os/os_linux.odin b/core/os/os_linux.odin index 29219dc3b..78da32a57 100644 --- a/core/os/os_linux.odin +++ b/core/os/os_linux.odin @@ -886,6 +886,12 @@ _readlink :: proc(path: string) -> (string, Error) { } } +@(private, require_results) +_dup :: proc(fd: Handle) -> (Handle, Error) { + dup, err := linux.dup(linux.Fd(fd)) + return Handle(dup), err +} + @(require_results) absolute_path_from_handle :: proc(fd: Handle) -> (string, Error) { buf : [256]byte diff --git a/core/os/os_netbsd.odin b/core/os/os_netbsd.odin index c41dc6aa6..ffaf4a5f8 100644 --- a/core/os/os_netbsd.odin +++ b/core/os/os_netbsd.odin @@ -441,6 +441,7 @@ foreign libc { @(link_name="rmdir") _unix_rmdir :: proc(path: cstring) -> c.int --- @(link_name="mkdir") _unix_mkdir :: proc(path: cstring, mode: mode_t) -> c.int --- @(link_name="fcntl") _unix_fcntl :: proc(fd: Handle, cmd: c.int, arg: uintptr) -> c.int --- + @(link_name="dup") _unix_dup :: proc(fd: Handle) -> Handle --- @(link_name="fdopendir") _unix_fdopendir :: proc(fd: Handle) -> Dir --- @(link_name="closedir") _unix_closedir :: proc(dirp: Dir) -> c.int --- @@ -801,6 +802,15 @@ _readlink :: proc(path: string) -> (string, Error) { return "", Error{} } +@(private, require_results) +_dup :: proc(fd: Handle) -> (Handle, Error) { + dup := _unix_dup(fd) + if dup == -1 { + return INVALID_HANDLE, get_last_error() + } + return dup, nil +} + @(require_results) absolute_path_from_handle :: proc(fd: Handle) -> (path: string, err: Error) { buf: [MAX_PATH]byte diff --git a/core/os/os_openbsd.odin b/core/os/os_openbsd.odin index 1cd26211e..a41637ea3 100644 --- a/core/os/os_openbsd.odin +++ b/core/os/os_openbsd.odin @@ -364,6 +364,7 @@ foreign libc { @(link_name="unlink") _unix_unlink :: proc(path: cstring) -> c.int --- @(link_name="rmdir") _unix_rmdir :: proc(path: cstring) -> c.int --- @(link_name="mkdir") _unix_mkdir :: proc(path: cstring, mode: mode_t) -> c.int --- + @(link_name="dup") _unix_dup :: proc(fd: Handle) -> Handle --- @(link_name="getpagesize") _unix_getpagesize :: proc() -> c.int --- @(link_name="sysconf") _sysconf :: proc(name: c.int) -> c.long --- @@ -716,6 +717,15 @@ _readlink :: proc(path: string) -> (string, Error) { } } +@(private, require_results) +_dup :: proc(fd: Handle) -> (Handle, Error) { + dup := _unix_dup(fd) + if dup == -1 { + return INVALID_HANDLE, get_last_error() + } + return dup, nil +} + // XXX OpenBSD @(require_results) absolute_path_from_handle :: proc(fd: Handle) -> (string, Error) { diff --git a/tests/core/os/os.odin b/tests/core/os/os.odin index 0beac67a6..1510bad31 100644 --- a/tests/core/os/os.odin +++ b/tests/core/os/os.odin @@ -32,7 +32,9 @@ read_dir :: proc(t: ^testing.T) { fd, err := os.open(#directory + "/dir") testing.expect_value(t, err, nil) - defer os.close(fd) + defer { + testing.expect_value(t, os.close(fd), nil) + } dir, err2 := os.read_dir(fd, -1) testing.expect_value(t, err2, nil) @@ -58,4 +60,4 @@ read_dir :: proc(t: ^testing.T) { testing.expect_value(t, dir[2].name, "sub") testing.expect(t, dir[2].is_dir, "is not a directory") } -} \ No newline at end of file +}