fix child process stdio piping behavior on windows

ziglang · Oct 15, 2017 · bb169a7 · bb169a7
1 parent 1fe1e6e
commit bb169a7
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 17 deletions.
diff --git a/std/io.zig b/std/io.zig
@@ -313,8 +313,8 @@ pub const InStream = struct {
                         else => error.Unexpected,
                     };
                 }
-                if (amt_read == 0) return index;
                 index += amt_read;
+                if (amt_read < want_read_count) return index;
             }
             return index;
         } else {

diff --git a/std/os/child_process.zig b/std/os/child_process.zig
@@ -433,7 +433,7 @@ pub const ChildProcess = struct {
     }
 
     fn spawnWindows(self: &ChildProcess) -> %void {
-        var saAttr = windows.SECURITY_ATTRIBUTES {
+        const saAttr = windows.SECURITY_ATTRIBUTES {
             .nLength = @sizeOf(windows.SECURITY_ATTRIBUTES),
             .bInheritHandle = windows.TRUE,
             .lpSecurityDescriptor = null,
@@ -459,7 +459,7 @@ pub const ChildProcess = struct {
         var g_hChildStd_IN_Wr: ?windows.HANDLE = null;
         switch (self.stdin_behavior) {
             StdIo.Pipe => {
-                %return windowsMakePipeIn(&g_hChildStd_IN_Rd, &g_hChildStd_IN_Wr, &saAttr);
+                %return windowsMakePipeIn(&g_hChildStd_IN_Rd, &g_hChildStd_IN_Wr, saAttr);
             },
             StdIo.Ignore => {
                 g_hChildStd_IN_Rd = nul_handle;
@@ -477,7 +477,7 @@ pub const ChildProcess = struct {
         var g_hChildStd_OUT_Wr: ?windows.HANDLE = null;
         switch (self.stdout_behavior) {
             StdIo.Pipe => {
-                %return windowsMakePipeOut(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, &saAttr);
+                %return windowsMakePipeOut(&g_hChildStd_OUT_Rd, &g_hChildStd_OUT_Wr, saAttr);
             },
             StdIo.Ignore => {
                 g_hChildStd_OUT_Wr = nul_handle;
@@ -495,7 +495,7 @@ pub const ChildProcess = struct {
         var g_hChildStd_ERR_Wr: ?windows.HANDLE = null;
         switch (self.stderr_behavior) {
             StdIo.Pipe => {
-                %return windowsMakePipeOut(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, &saAttr);
+                %return windowsMakePipeOut(&g_hChildStd_ERR_Rd, &g_hChildStd_ERR_Wr, saAttr);
             },
             StdIo.Ignore => {
                 g_hChildStd_ERR_Wr = nul_handle;
@@ -675,7 +675,12 @@ fn windowsDestroyPipe(rd: ?windows.HANDLE, wr: ?windows.HANDLE) {
     if (wr) |h| os.windowsClose(h);
 }
 
-fn windowsMakePipe(rd: &windows.HANDLE, wr: &windows.HANDLE, sattr: &windows.SECURITY_ATTRIBUTES) -> %void {
+
+// TODO: workaround for bug where the `const` from `&const` is dropped when the type is
+// a namespace field lookup
+const SECURITY_ATTRIBUTES = windows.SECURITY_ATTRIBUTES;
+
+fn windowsMakePipe(rd: &windows.HANDLE, wr: &windows.HANDLE, sattr: &const SECURITY_ATTRIBUTES) -> %void {
     if (windows.CreatePipe(rd, wr, sattr, 0) == 0) {
         const err = windows.GetLastError();
         return switch (err) {
@@ -693,19 +698,21 @@ fn windowsSetHandleInfo(h: windows.HANDLE, mask: windows.DWORD, flags: windows.D
     }
 }
 
-fn windowsMakePipeIn(rd: &?windows.HANDLE, wr: &?windows.HANDLE, sattr: &windows.SECURITY_ATTRIBUTES) -> %void {
+fn windowsMakePipeIn(rd: &?windows.HANDLE, wr: &?windows.HANDLE, sattr: &const SECURITY_ATTRIBUTES) -> %void {
     var rd_h: windows.HANDLE = undefined;
     var wr_h: windows.HANDLE = undefined;
     %return windowsMakePipe(&rd_h, &wr_h, sattr);
+    %defer windowsDestroyPipe(rd_h, wr_h);
     %return windowsSetHandleInfo(wr_h, windows.HANDLE_FLAG_INHERIT, 0);
     *rd = rd_h;
     *wr = wr_h;
 }
 
-fn windowsMakePipeOut(rd: &?windows.HANDLE, wr: &?windows.HANDLE, sattr: &windows.SECURITY_ATTRIBUTES) -> %void {
+fn windowsMakePipeOut(rd: &?windows.HANDLE, wr: &?windows.HANDLE, sattr: &const SECURITY_ATTRIBUTES) -> %void {
     var rd_h: windows.HANDLE = undefined;
     var wr_h: windows.HANDLE = undefined;
     %return windowsMakePipe(&rd_h, &wr_h, sattr);
+    %defer windowsDestroyPipe(rd_h, wr_h);
     %return windowsSetHandleInfo(rd_h, windows.HANDLE_FLAG_INHERIT, 0);
     *rd = rd_h;
     *wr = wr_h;

diff --git a/std/os/windows/index.zig b/std/os/windows/index.zig
@@ -18,7 +18,7 @@ pub extern "kernel32" stdcallcc fn CreateFileA(lpFileName: LPCSTR, dwDesiredAcce
         dwFlagsAndAttributes: DWORD, hTemplateFile: ?HANDLE) -> HANDLE;
 
 pub extern "kernel32" stdcallcc fn CreatePipe(hReadPipe: &HANDLE, hWritePipe: &HANDLE,
-    lpPipeAttributes: &SECURITY_ATTRIBUTES, nSize: DWORD) -> BOOL;
+    lpPipeAttributes: &const SECURITY_ATTRIBUTES, nSize: DWORD) -> BOOL;
 
 pub extern "kernel32" stdcallcc fn CreateProcessA(lpApplicationName: ?LPCSTR, lpCommandLine: LPSTR,
     lpProcessAttributes: ?&SECURITY_ATTRIBUTES, lpThreadAttributes: ?&SECURITY_ATTRIBUTES, bInheritHandles: BOOL,

diff --git a/test/tests.zig b/test/tests.zig
@@ -557,7 +557,7 @@ pub const CompileErrorContext = struct {
             %%io.stderr.printf("Test {}/{} {}...", self.test_index+1, self.context.test_index, self.name);
 
             if (b.verbose) {
-                printInvocation(b.zig_exe, zig_args.toSliceConst());
+                printInvocation(zig_args.toSliceConst());
             }
 
             const child = %%os.ChildProcess.init(zig_args.toSliceConst(), b.allocator);
@@ -625,10 +625,9 @@ pub const CompileErrorContext = struct {
         }
     };
 
-    fn printInvocation(exe_path: []const u8, args: []const []const u8) {
-        %%io.stderr.printf("{}", exe_path);
+    fn printInvocation(args: []const []const u8) {
         for (args) |arg| {
-            %%io.stderr.printf(" {}", arg);
+            %%io.stderr.printf("{} ", arg);
         }
         %%io.stderr.printf("\n");
     }
@@ -826,7 +825,7 @@ pub const ParseCContext = struct {
             %%io.stderr.printf("Test {}/{} {}...", self.test_index+1, self.context.test_index, self.name);
 
             if (b.verbose) {
-                printInvocation(b.zig_exe, zig_args.toSliceConst());
+                printInvocation(zig_args.toSliceConst());
             }
 
             const child = %%os.ChildProcess.init(zig_args.toSliceConst(), b.allocator);
@@ -895,10 +894,9 @@ pub const ParseCContext = struct {
         }
     };
 
-    fn printInvocation(exe_path: []const u8, args: []const []const u8) {
-        %%io.stderr.printf("{}", exe_path);
+    fn printInvocation(args: []const []const u8) {
         for (args) |arg| {
-            %%io.stderr.printf(" {}", arg);
+            %%io.stderr.printf("{} ", arg);
         }
         %%io.stderr.printf("\n");
     }