Merge pull request #4013 from laytan/fix-os-read-dir-with-symlinks

fix os.read_dir with symlinks

core/os/dir_bsd.odin

@@ -1,73 +0,0 @@
-//+build freebsd, netbsd
-package os
-
-import "core:mem"
-
-read_dir :: proc(fd: Handle, n: int, allocator := context.allocator) -> (fi: []File_Info, err: Errno) {
-	dirp: Dir
-	dirp, err = _fdopendir(fd)
-	if err != ERROR_NONE {
-		return
-	}
-
-	defer _closedir(dirp)
-
-	dirpath: string
-	dirpath, err = absolute_path_from_handle(fd)
-
-	if err != ERROR_NONE {
-		return
-	}
-
-	defer delete(dirpath)
-
-	n := n
-	size := n
-	if n <= 0 {
-		n = -1
-		size = 100
-	}
-
-	dfi := make([dynamic]File_Info, 0, size, allocator)
-
-	for {
-		entry: Dirent
-		end_of_stream: bool
-		entry, err, end_of_stream = _readdir(dirp)
-		if err != ERROR_NONE {
-			for fi_ in dfi {
-				file_info_delete(fi_, allocator)
-			}
-			delete(dfi)
-			return
-		} else if end_of_stream {
-			break
-		}
-
-		fi_: File_Info
-		filename := cast(string)(transmute(cstring)mem.Raw_Cstring{ data = &entry.name[0] })
-
-		if filename == "." || filename == ".." {
-			continue
-		}
-
-		fullpath := make([]byte, len(dirpath)+1+len(filename), context.temp_allocator)
-		copy(fullpath, dirpath)
-		copy(fullpath[len(dirpath):], "/")
-		copy(fullpath[len(dirpath)+1:], filename)
-		defer delete(fullpath, context.temp_allocator)
-
-		fi_, err = stat(string(fullpath), allocator)
-		if err != ERROR_NONE {
-			for fi__ in dfi {
-				file_info_delete(fi__, allocator)
-			}
-			delete(dfi)
-			return
-		}
-
-		append(&dfi, fi_)
-	}
-
-	return dfi[:], ERROR_NONE
-}

core/os/dir_linux.odin

@@ -1,72 +0,0 @@
-package os
-
-import "core:strings"
-import "core:mem"
-import "base:runtime"
-
-read_dir :: proc(fd: Handle, n: int, allocator := context.allocator) -> (fi: []File_Info, err: Errno) {
-	dirp: Dir
-	dirp, err = _fdopendir(fd)
-	if err != ERROR_NONE {
-		return
-	}
-
-	defer _closedir(dirp)
-
-	dirpath: string
-	dirpath, err = absolute_path_from_handle(fd)
-
-	if err != ERROR_NONE {
-		return
-	}
-
-	defer delete(dirpath)
-
-	n := n
-	size := n
-	if n <= 0 {
-		n = -1
-		size = 100
-	}
-
-	dfi := make([dynamic]File_Info, 0, size, allocator)
-
-	for {
-		entry: Dirent
-		end_of_stream: bool
-		entry, err, end_of_stream = _readdir(dirp)
-		if err != ERROR_NONE {
-			for fi_ in dfi {
-				file_info_delete(fi_, allocator)
-			}
-			delete(dfi)
-			return
-		} else if end_of_stream {
-			break
-		}
-
-		fi_: File_Info
-		filename := cast(string)(transmute(cstring)mem.Raw_Cstring{ data = &entry.name[0] })
-
-		if filename == "." || filename == ".." {
-			continue
-		}
-
-		runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD(ignore = context.temp_allocator == allocator)
-		fullpath := strings.join( []string{ dirpath, filename }, "/", context.temp_allocator)
-		defer delete(fullpath, context.temp_allocator)
-
-		fi_, err = stat(fullpath, allocator)
-		if err != ERROR_NONE {
-			for fi__ in dfi {
-				file_info_delete(fi__, allocator)
-			}
-			delete(dfi)
-			return
-		}
-
-		append(&dfi, fi_)
-	}
-
-	return dfi[:], ERROR_NONE
-}

core/os/dir_openbsd.odin

@@ -1,71 +0,0 @@
-package os
-
-import "core:strings"
-import "core:mem"
-
-read_dir :: proc(fd: Handle, n: int, allocator := context.allocator) -> (fi: []File_Info, err: Errno) {
-	dirp: Dir
-	dirp, err = _fdopendir(fd)
-	if err != ERROR_NONE {
-		return
-	}
-
-	defer _closedir(dirp)
-
-	// XXX OpenBSD
-	dirpath: string
-	dirpath, err = absolute_path_from_handle(fd)
-
-	if err != ERROR_NONE {
-		return
-	}
-
-	defer delete(dirpath)
-
-	n := n
-	size := n
-	if n <= 0 {
-		n = -1
-		size = 100
-	}
-
-	dfi := make([dynamic]File_Info, 0, size, allocator)
-
-	for {
-		entry: Dirent
-		end_of_stream: bool
-		entry, err, end_of_stream = _readdir(dirp)
-		if err != ERROR_NONE {
-			for fi_ in dfi {
-				file_info_delete(fi_, allocator)
-			}
-			delete(dfi)
-			return
-		} else if end_of_stream {
-			break
-		}
-
-		fi_: File_Info
-		filename := cast(string)(transmute(cstring)mem.Raw_Cstring{ data = &entry.name[0] })
-
-		if filename == "." || filename == ".." {
-			continue
-		}
-
-		fullpath := strings.join( []string{ dirpath, filename }, "/", context.temp_allocator)
-		defer delete(fullpath, context.temp_allocator)
-
-		fi_, err = stat(fullpath, allocator)
-		if err != ERROR_NONE {
-			for fi__ in dfi {
-				file_info_delete(fi__, allocator)
-			}
-			delete(dfi)
-			return
-		}
-
-		append(&dfi, fi_)
-	}
-
-	return dfi[:], ERROR_NONE
-}

core/os/dir_unix.odin

@@ -1,7 +1,7 @@
+//+build darwin, linux, netbsd, freebsd, openbsd
 package os
 
 import "core:strings"
-import "core:mem"
 
 read_dir :: proc(fd: Handle, n: int, allocator := context.allocator) -> (fi: []File_Info, err: Errno) {
 	dirp: Dir
@@ -28,39 +28,41 @@ read_dir :: proc(fd: Handle, n: int, allocator := context.allocator) -> (fi: []F
 	}
 
 	dfi := make([dynamic]File_Info, 0, size, allocator)
+	defer if err != ERROR_NONE {
+		for fi_ in dfi {
+			file_info_delete(fi_, allocator)
+		}
+		delete(dfi)
+	}
 
 	for {
 		entry: Dirent
 		end_of_stream: bool
 		entry, err, end_of_stream = _readdir(dirp)
 		if err != ERROR_NONE {
-			for fi_ in dfi {
-				file_info_delete(fi_, allocator)
-			}
-			delete(dfi)
 			return
 		} else if end_of_stream {
 			break
 		}
 
 		fi_: File_Info
-		filename := cast(string)(transmute(cstring)mem.Raw_Cstring{ data = &entry.name[0] })
+		filename := string(cstring(&entry.name[0]))
 
 		if filename == "." || filename == ".." {
 			continue
 		}
 
-		fullpath := strings.join( []string{ dirpath, filename }, "/", context.temp_allocator)
+		fullpath := strings.join({ dirpath, filename }, "/", allocator)
-		defer delete(fullpath, context.temp_allocator)
 
-		fi_, err = stat(fullpath, allocator)
+		s: OS_Stat
+		s, err = _lstat(fullpath)
 		if err != ERROR_NONE {
-			for fi__ in dfi {
+			delete(fullpath, allocator)
-				file_info_delete(fi__, allocator)
-			}
-			delete(dfi)
 			return
 		}
+		_fill_file_info_from_stat(&fi_, s)
+		fi_.fullpath = fullpath
+		fi_.name = path_base(fi_.fullpath)
 
 		append(&dfi, fi_)
 	}

tests/core/normal.odin

@@ -30,6 +30,7 @@ download_assets :: proc() {
 @(require) import "mem"
 @(require) import "net"
 @(require) import "odin"
+@(require) import "os"
 @(require) import "path/filepath"
 @(require) import "reflect"
 @(require) import "runtime"

tests/core/os/dir/alink.txt

@@ -0,0 +1 @@
+./a.txt

tests/core/os/os.odin

@@ -0,0 +1,35 @@
+package tests_core_os
+
+import "core:os"
+import "core:slice"
+
+import "core:testing"
+
+@(test)
+read_dir :: proc(t: ^testing.T) {
+	fd, errno := os.open(#directory + "/dir")
+	testing.expect_value(t, errno, os.ERROR_NONE)
+	defer os.close(fd)
+
+	dir, errno2 := os.read_dir(fd, -1)
+	testing.expect_value(t, errno2, os.ERROR_NONE)
+	defer os.file_info_slice_delete(dir)
+
+	slice.sort_by_key(dir, proc(fi: os.File_Info) -> string { return fi.name })
+
+	testing.expect_value(t, len(dir), 3)
+
+	testing.expect_value(t, dir[0].name, "alink.txt")
+	testing.expect(t, !dir[0].is_dir, "is a directory")
+	when ODIN_OS == .Windows {
+		testing.expect(t, dir[0].mode & os.File_Mode_Sym_Link != 0, "not a symlink")
+	} else {
+		testing.expect(t, os.S_ISLNK(auto_cast dir[0].mode), "not a symlink")
+	}
+
+	testing.expect_value(t, dir[1].name, "b.txt")
+
+	testing.expect_value(t, dir[2].name, "sub")
+	testing.expect(t, dir[2].is_dir, "is not a directory")
+}
+