adding probing, if inheritance is enabled + cleanups

Author: Jan Philipp Hafer

lib/std/child_process.zig

@@ -1108,11 +1108,14 @@ pub const ChildProcess = struct {
     }
 };
 
+/// Pipe read side
+pub const pipe_rd = 0;
+/// Pipe write side
+pub const pipe_wr = 1;
 const PortPipeT = if (builtin.os.tag == .windows) [2]windows.HANDLE else [2]os.fd_t;
 
 /// Portable pipe creation with disabled inheritance
 pub inline fn portablePipe() !PortPipeT {
-    // TODO think how to offer user an interface to lpSecurityDescriptor
     var pipe_new: PortPipeT = undefined;
     if (builtin.os.tag == .windows) {
         const saAttr = windows.SECURITY_ATTRIBUTES{

lib/std/os.zig

@@ -44,32 +44,6 @@ pub const windows = @import("os/windows.zig");
 pub const posix_spawn = @import("os/posix_spawn.zig");
 pub const ptrace = @import("os/ptrace.zig");
 
-/// Pipe read side
-pub const pipe_rd = 0;
-/// Pipe write side
-pub const pipe_wr = 1;
-
-pub const windowsPtrDigits: usize = std.math.log10(math.maxInt(usize));
-pub const otherPtrDigits: usize = std.math.log10(math.maxInt(u32)) + 1; // +1 for sign
-pub const handleCharSize = if (builtin.target.os.tag == .windows) windowsPtrDigits else otherPtrDigits;
-
-pub fn handleToString(handle: fd_t, buf: []u8) std.fmt.BufPrintError![]u8 {
-    var s_handle: []u8 = undefined;
-    const handle_int =
-        // handle is *anyopaque or an integer on unix-likes Kernels.
-        if (builtin.target.os.tag == .windows) @ptrToInt(handle) else handle;
-    s_handle = try std.fmt.bufPrint(buf[0..], "{d}", .{handle_int});
-    return s_handle;
-}
-
-pub fn stringToHandle(s_handle: []const u8) std.fmt.ParseIntError!std.os.fd_t {
-    var file_handle: std.os.fd_t = if (builtin.target.os.tag == .windows)
-        @intToPtr(windows.HANDLE, try std.fmt.parseInt(usize, s_handle, 10))
-    else
-        try std.fmt.parseInt(std.os.fd_t, s_handle, 10);
-    return file_handle;
-}
-
 comptime {
     assert(@import("std") == std); // std lib tests require --zig-lib-dir
 }
@@ -314,6 +288,27 @@ pub fn close(fd: fd_t) void {
     }
 }
 
+pub const windowsPtrDigits = 19; // log10(max(usize))
+pub const unixoidPtrDigits = 10; // log10(max(u32)) + 1 for sign
+pub const handleCharSize = if (builtin.target.os.tag == .windows) windowsPtrDigits else unixoidPtrDigits;
+
+pub fn handleToString(handle: fd_t, buf: []u8) std.fmt.BufPrintError![]u8 {
+    var s_handle: []u8 = undefined;
+    const handle_int =
+        // handle is *anyopaque or an integer on unix-likes Kernels.
+        if (builtin.target.os.tag == .windows) @ptrToInt(handle) else handle;
+    s_handle = try std.fmt.bufPrint(buf[0..], "{d}", .{handle_int});
+    return s_handle;
+}
+
+pub fn stringToHandle(s_handle: []const u8) std.fmt.ParseIntError!std.os.fd_t {
+    var handle: std.os.fd_t = if (builtin.target.os.tag == .windows)
+        @intToPtr(windows.HANDLE, try std.fmt.parseInt(usize, s_handle, 10))
+    else
+        try std.fmt.parseInt(std.os.fd_t, s_handle, 10);
+    return handle;
+}
+
 pub const FChmodError = error{
     AccessDenied,
     InputOutput,
@@ -4869,25 +4864,41 @@ pub fn lseek_CUR_get(fd: fd_t) SeekError!u64 {
     }
 }
 
+const IsInheritableError = FcntlError || windows.GetHandleInformationError;
+
+/// Whether inheritence is enabled or CLOEXEC is not set.
+pub inline fn isInheritable(handle: fd_t) IsInheritableError!bool {
+    if (builtin.os.tag == .windows) {
+        var handle_flags: windows.DWORD = undefined;
+        try windows.GetHandleInformation(handle, &handle_flags);
+        return handle_flags & windows.HANDLE_FLAG_INHERIT != 0;
+    } else {
+        const fcntl_flags = try fcntl(handle, F.GETFD, 0);
+        return fcntl_flags & FD_CLOEXEC == 0;
+    }
+}
+
 const EnableInheritanceError = FcntlError || windows.SetHandleInformationError;
 
-pub inline fn enableInheritance(file_handle: fd_t) EnableInheritanceError!void {
+/// Enables inheritence or sets CLOEXEC.
+pub inline fn enableInheritance(handle: fd_t) EnableInheritanceError!void {
     if (builtin.os.tag == .windows) {
-        try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 1);
+        try windows.SetHandleInformation(handle, windows.HANDLE_FLAG_INHERIT, 1);
     } else {
-        var flags = try fcntl(file_handle, F.GETFD, 0);
+        var flags = try fcntl(handle, F.GETFD, 0);
         flags &= ~@as(u32, FD_CLOEXEC);
-        _ = try fcntl(file_handle, F.SETFD, flags);
+        _ = try fcntl(handle, F.SETFD, flags);
     }
 }
 
 const DisableInheritanceError = FcntlError || windows.SetHandleInformationError;
 
-pub inline fn disableInheritance(file_handle: fd_t) DisableInheritanceError!void {
+/// Disables inheritence or unsets CLOEXEC.
+pub inline fn disableInheritance(handle: fd_t) DisableInheritanceError!void {
     if (builtin.os.tag == .windows) {
-        try windows.SetHandleInformation(file_handle, windows.HANDLE_FLAG_INHERIT, 0);
+        try windows.SetHandleInformation(handle, windows.HANDLE_FLAG_INHERIT, 0);
     } else {
-        _ = try fcntl(file_handle, F.SETFD, FD_CLOEXEC);
+        _ = try fcntl(handle, F.SETFD, FD_CLOEXEC);
     }
 }
 

test/standalone/childprocess_extrapipe/child.zig

@@ -12,16 +12,10 @@ pub fn main() !void {
     const s_handle = it.next() orelse unreachable;
     var file_handle = try std.os.stringToHandle(s_handle);
 
-    if (builtin.target.os.tag == .windows) {
-        // windows.HANDLE_FLAG_INHERIT is enabled
-        var handle_flags: windows.DWORD = undefined;
-        try windows.GetHandleInformation(file_handle, &handle_flags);
-        try std.testing.expect(handle_flags & windows.HANDLE_FLAG_INHERIT != 0);
-    } else {
-        // FD_CLOEXEC is not set
-        var fcntl_flags = try std.os.fcntl(file_handle, std.os.F.GETFD, 0);
-        try std.testing.expect((fcntl_flags & std.os.FD_CLOEXEC) == 0);
-    }
+    // child inherited the handle, so inheritance must be enabled
+    const is_inheritable = try std.os.isInheritable(file_handle);
+    std.debug.assert(is_inheritable);
+
     try std.os.disableInheritance(file_handle);
     var file_in = std.fs.File{ .handle = file_handle }; // read side of pipe
     defer file_in.close();

test/standalone/childprocess_extrapipe/parent.zig

@@ -6,8 +6,8 @@ const windows = std.os.windows;
 const os = std.os;
 const testing = std.testing;
 const child_process = std.child_process;
-const pipe_rd = os.pipe_rd;
-const pipe_wr = os.pipe_wr;
+const pipe_rd = child_process.pipe_rd;
+const pipe_wr = child_process.pipe_wr;
 
 pub fn main() !void {
     var gpa_state = std.heap.GeneralPurposeAllocator(.{}){};
@@ -39,14 +39,8 @@ pub fn main() !void {
     }
 
     // check that inheritance was disabled for the handle the whole time
-    if (builtin.target.os.tag == .windows) {
-        var handle_flags: windows.DWORD = undefined;
-        try windows.GetHandleInformation(pipe[pipe_wr], &handle_flags);
-        std.debug.assert(handle_flags & windows.HANDLE_FLAG_INHERIT == 0);
-    } else {
-        const fcntl_flags = try os.fcntl(pipe[pipe_wr], os.F.GETFD, 0);
-        try std.testing.expect((fcntl_flags & os.FD_CLOEXEC) != 0);
-    }
+    const is_inheritable = try os.isInheritable(pipe[pipe_wr]);
+    std.debug.assert(!is_inheritable);
 
     var file_out = std.fs.File{ .handle = pipe[pipe_wr] };
     defer file_out.close();