Merge pull request #4142 from laytan/os2-propogate-errors-from-execv

os2: propagate errors from execv

4 files changed, 14 insertions, 25 deletions

diff --git a/core/os/os2/file_util.odin b/core/os/os2/file_util.odin
index e328f9a02..963544985 100644
--- a/core/os/os2/file_util.odin
+++ b/core/os/os2/file_util.odin
@@ -125,9 +125,6 @@ read_entire_file_from_file :: proc(f: ^File, allocator: runtime.Allocator) -> (d
 			has_size = true
 			size = int(size64)
 		}
-	} else if serr != .No_Size {
-		err = serr
-		return
 	}
 
 	if has_size && size > 0 {
diff --git a/core/os/os2/process.odin b/core/os/os2/process.odin
index f7a542276..1310af0d9 100644
--- a/core/os/os2/process.odin
+++ b/core/os/os2/process.odin
@@ -305,6 +305,7 @@ Process_Desc :: struct {
 	// A slice of strings, each having the format `KEY=VALUE` representing the
 	// full environment that the child process will receive.
 	// In case this slice is `nil`, the current process' environment is used.
+	// NOTE(laytan): maybe should be `Maybe([]string)` so you can do `nil` == current env, empty == empty/no env.
 	env: []string,
 	// The `stderr` handle to give to the child process. It can be either a file
 	// or a writeable end of a pipe. Passing `nil` will shut down the process'
diff --git a/core/os/os2/process_linux.odin b/core/os/os2/process_linux.odin
index 40406bad5..b6db46423 100644
--- a/core/os/os2/process_linux.odin
+++ b/core/os/os2/process_linux.odin
@@ -490,7 +490,6 @@ _process_start :: proc(desc: Process_Desc) -> (process: Process, err: Error) {
 	if errno = linux.pipe2(&child_pipe_fds, {.CLOEXEC}); errno != .NONE {
 		return process, _get_platform_error(errno)
 	}
-	defer linux.close(child_pipe_fds[WRITE])
 	defer linux.close(child_pipe_fds[READ])
 
 
@@ -508,6 +507,7 @@ _process_start :: proc(desc: Process_Desc) -> (process: Process, err: Error) {
 	//
 	pid: linux.Pid
 	if pid, errno = linux.fork(); errno != .NONE {
+		linux.close(child_pipe_fds[WRITE])
 		return process, _get_platform_error(errno)
 	}
 
@@ -573,25 +573,19 @@ _process_start :: proc(desc: Process_Desc) -> (process: Process, err: Error) {
 			write_errno_to_parent_and_abort(child_pipe_fds[WRITE], errno)
 		}
 
-		success_byte: [1]u8
-		linux.write(child_pipe_fds[WRITE], success_byte[:])
-
 		errno = linux.execveat(exe_fd, "", &cargs[0], env, {.AT_EMPTY_PATH})
-
-		// NOTE: we can't tell the parent about this failure because we already wrote the success byte.
-		// So if this happens the user will just see the process failed when they call process_wait.
-
 		assert(errno != nil)
-		intrinsics.trap()
+		write_errno_to_parent_and_abort(child_pipe_fds[WRITE], errno)
 	}
 
+	linux.close(child_pipe_fds[WRITE])
+
 	process.pid = int(pid)
 
-	n: int
 	child_byte: [1]u8
 	errno = .EINTR
 	for errno == .EINTR {
-		n, errno = linux.read(child_pipe_fds[READ], child_byte[:])
+		_, errno = linux.read(child_pipe_fds[READ], child_byte[:])
 	}
 
 	// If the read failed, something weird happened. Do not return the read
diff --git a/core/os/os2/process_posix.odin b/core/os/os2/process_posix.odin
index c9b67f199..ea4ada81e 100644
--- a/core/os/os2/process_posix.odin
+++ b/core/os/os2/process_posix.odin
@@ -139,20 +139,22 @@ _process_start :: proc(desc: Process_Desc) -> (process: Process, err: Error) {
 		err = _get_platform_error()
 		return
 	}
-	defer posix.close(pipe[WRITE])
 	defer posix.close(pipe[READ])
 
 	if posix.fcntl(pipe[READ], .SETFD, i32(posix.FD_CLOEXEC)) == -1 {
+		posix.close(pipe[WRITE])
 		err = _get_platform_error()
 		return
 	}
 	if posix.fcntl(pipe[WRITE], .SETFD, i32(posix.FD_CLOEXEC)) == -1 {
+		posix.close(pipe[WRITE])
 		err = _get_platform_error()
 		return
 	}
 
 	switch pid := posix.fork(); pid {
 	case -1:
+		posix.close(pipe[WRITE])
 		err = _get_platform_error()
 		return
 
@@ -179,25 +181,20 @@ _process_start :: proc(desc: Process_Desc) -> (process: Process, err: Error) {
 			if posix.chdir(cwd) != .OK { abort(pipe[WRITE]) }
 		}
 
-		ok := u8(0)
-		posix.write(pipe[WRITE], &ok, 1)
-
 		res := posix.execve(strings.to_cstring(&exe_builder), raw_data(cmd), env)
-
-		// NOTE: we can't tell the parent about this failure because we already wrote the success byte.
-		// So if this happens the user will just see the process failed when they call process_wait.
-
 		assert(res == -1)
-		runtime.trap()
+		abort(pipe[WRITE])
 
 	case:
+		posix.close(pipe[WRITE])
+
 		errno: posix.Errno
 		for {
 			errno_byte: u8
 			switch posix.read(pipe[READ], &errno_byte, 1) {
-			case 1:
+			case  1:
 				errno = posix.Errno(errno_byte)
-			case:
+			case -1:
 				errno = posix.errno()
 				if errno == .EINTR {
 					continue