Merge pull request #4489 from laytan/os2-dir-leak-and-test

os2: fix leak in dir_windows, fix netbsd, and add a test for dir reading

core/os/os2/dir_linux.odin

@@ -13,7 +13,7 @@ _read_directory_iterator :: proc(it: ^Read_Directory_Iterator) -> (fi: File_Info
 
 @(require_results)
 _read_directory_iterator_create :: proc(f: ^File) -> (Read_Directory_Iterator, Error) {
-	return {}, nil
+	return {}, .Unsupported
 }
 
 _read_directory_iterator_destroy :: proc(it: ^Read_Directory_Iterator) {

core/os/os2/dir_windows.odin

@@ -16,28 +16,25 @@ find_data_to_file_info :: proc(base_path: string, d: ^win32.WIN32_FIND_DATAW, al
 	}
 	path := concatenate({base_path, `\`, win32_utf16_to_utf8(d.cFileName[:], temp_allocator()) or_else ""}, allocator) or_return
 
+	handle := win32.HANDLE(_open_internal(path, {.Read}, 0o666) or_else 0)
+	defer win32.CloseHandle(handle)
 
 	fi.fullpath = path
 	fi.name = basename(path)
 	fi.size = i64(d.nFileSizeHigh)<<32 + i64(d.nFileSizeLow)
 
-	fi.type, fi.mode = _file_type_mode_from_file_attributes(d.dwFileAttributes, nil, d.dwReserved0)
+	fi.type, fi.mode = _file_type_mode_from_file_attributes(d.dwFileAttributes, handle, d.dwReserved0)
 
 	fi.creation_time     = time.unix(0, win32.FILETIME_as_unix_nanoseconds(d.ftCreationTime))
 	fi.modification_time = time.unix(0, win32.FILETIME_as_unix_nanoseconds(d.ftLastWriteTime))
 	fi.access_time       = time.unix(0, win32.FILETIME_as_unix_nanoseconds(d.ftLastAccessTime))
 
-
-	handle := win32.HANDLE(_open_internal(path, {.Read}, 0o666) or_else 0)
-	defer win32.CloseHandle(handle)
-
 	if file_id_info: win32.FILE_ID_INFO; handle != nil && win32.GetFileInformationByHandleEx(handle, .FileIdInfo, &file_id_info, size_of(file_id_info)) {
 		#assert(size_of(fi.inode) == size_of(file_id_info.FileId))
 		#assert(size_of(fi.inode) == 16)
 		runtime.mem_copy_non_overlapping(&fi.inode, &file_id_info.FileId, 16)
 	}
 
-
 	return
 }
 
@@ -137,5 +134,6 @@ _read_directory_iterator_destroy :: proc(it: ^Read_Directory_Iterator) {
 		return
 	}
 	file_info_delete(it.impl.prev_fi, file_allocator())
+	delete(it.impl.path, file_allocator())
 	win32.FindClose(it.impl.find_handle)
 }

core/os/os2/stat_windows.odin

@@ -200,22 +200,21 @@ _file_type_mode_from_file_attributes :: proc(file_attributes: win32.DWORD, h: wi
 	} else {
 		mode |= 0o666
 	}
+
 	is_sym := false
 	if file_attributes & win32.FILE_ATTRIBUTE_REPARSE_POINT == 0 {
 		is_sym = false
 	} else {
 		is_sym = ReparseTag == win32.IO_REPARSE_TAG_SYMLINK || ReparseTag == win32.IO_REPARSE_TAG_MOUNT_POINT
 	}
+
 	if is_sym {
 		type = .Symlink
-	} else {
+	} else if file_attributes & win32.FILE_ATTRIBUTE_DIRECTORY != 0 {
-		if file_attributes & win32.FILE_ATTRIBUTE_DIRECTORY != 0 {
+		type = .Directory
-			type = .Directory
+		mode |= 0o111
-			mode |= 0o111
+	} else if h != nil {
-		}
+		type = file_type(h)
-		if h != nil {
-			type = file_type(h)
-		}
 	}
 	return
 }

core/sys/posix/dirent.odin

@@ -54,15 +54,6 @@ foreign lib {
 	*/
 	closedir :: proc(dirp: DIR) -> result ---
 
-	/*
-	Return a file descriptor referring to the same directory as the dirp argument.
-
-	// TODO: this is a macro on NetBSD?
-
-	[[ More; https://pubs.opengroup.org/onlinepubs/9699919799/functions/dirfd.html ]]
-	*/
-	dirfd :: proc(dirp: DIR) -> FD ---
-
 	/*
 	Equivalent to the opendir() function except that the directory is specified by a file descriptor
 	rather than by a name.
@@ -161,11 +152,36 @@ when ODIN_OS == .NetBSD {
 	@(private) LSCANDIR   :: "__scandir30"
 	@(private) LOPENDIR   :: "__opendir30"
 	@(private) LREADDIR   :: "__readdir30"
+
+	/*
+	Return a file descriptor referring to the same directory as the dirp argument.
+
+	[[ More; https://pubs.opengroup.org/onlinepubs/9699919799/functions/dirfd.html ]]
+	*/
+	dirfd :: proc "c" (dirp: DIR) -> FD {
+		_dirdesc :: struct {
+			dd_fd: FD,
+
+			// more stuff...
+		}
+
+		return (^_dirdesc)(dirp).dd_fd
+	}
+
 } else {
 	@(private) LALPHASORT :: "alphasort" + INODE_SUFFIX
 	@(private) LSCANDIR   :: "scandir"   + INODE_SUFFIX
 	@(private) LOPENDIR   :: "opendir"   + INODE_SUFFIX
 	@(private) LREADDIR   :: "readdir"   + INODE_SUFFIX
+
+	foreign lib {
+		/*
+		Return a file descriptor referring to the same directory as the dirp argument.
+
+		[[ More; https://pubs.opengroup.org/onlinepubs/9699919799/functions/dirfd.html ]]
+		*/
+		dirfd :: proc(dirp: DIR) -> FD ---
+	}
 }
 
 when ODIN_OS == .Darwin {

tests/core/os/os2/dir.odin

@@ -0,0 +1,32 @@
+package tests_core_os_os2
+
+import os "core:os/os2"
+import    "core:log"
+import    "core:path/filepath"
+import    "core:slice"
+import    "core:testing"
+
+@(test)
+test_read_dir :: proc(t: ^testing.T) {
+	path := filepath.join({#directory, "../dir"})
+	defer delete(path)
+
+	fis, err := os.read_all_directory_by_path(path, context.allocator)
+	defer os.file_info_slice_delete(fis, context.allocator)
+
+	slice.sort_by_key(fis, proc(fi: os.File_Info) -> string { return fi.name })
+
+	if err == .Unsupported {
+		log.warn("os2 directory functionality is unsupported, skipping test")
+		return
+	}
+
+	testing.expect_value(t, err, nil)
+	testing.expect_value(t, len(fis), 2)
+
+	testing.expect_value(t, fis[0].name, "b.txt")
+	testing.expect_value(t, fis[0].type, os.File_Type.Regular)
+
+	testing.expect_value(t, fis[1].name, "sub")
+	testing.expect_value(t, fis[1].type, os.File_Type.Directory)
+}