diff --git a/src/stdlib_system_subprocess.c b/src/stdlib_system_subprocess.c index fc588214c..a3885cbb0 100644 --- a/src/stdlib_system_subprocess.c +++ b/src/stdlib_system_subprocess.c @@ -34,9 +34,10 @@ void process_create_windows(const char* cmd, const char* stdin_stream, STARTUPINFO si; PROCESS_INFORMATION pi; - HANDLE hStdout = NULL, hStderr = NULL; + HANDLE hStdout = NULL, hStderr = NULL, hStdin = NULL; SECURITY_ATTRIBUTES sa = { sizeof(SECURITY_ATTRIBUTES), NULL, TRUE }; FILE* stdin_fp = NULL; + char* full_cmd = NULL; // Initialize null handle (*pid) = 0; @@ -44,9 +45,6 @@ void process_create_windows(const char* cmd, const char* stdin_stream, ZeroMemory(&si, sizeof(si)); si.cb = sizeof(STARTUPINFO); - // If possible, we redirect stdout/stderr to file handles directly. - // This will override any cmd redirection settings (<>). For stdin - // Write stdin_stream to stdin_file if provided if (stdin_stream && stdin_file) { stdin_fp = fopen(stdin_file, "w"); @@ -58,57 +56,68 @@ void process_create_windows(const char* cmd, const char* stdin_stream, fclose(stdin_fp); } + // Open stdin file if provided, otherwise use the null device + if (stdin_file) { + hStdin = CreateFile(stdin_file, GENERIC_READ, 0, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + } else { + hStdin = CreateFile("NUL", GENERIC_READ, 0, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + } + + if (hStdin == INVALID_HANDLE_VALUE) { + fprintf(stderr, "Failed to open input source (file or null)\n"); + // No handles to close yet + return; + } + + si.hStdInput = hStdin; + si.dwFlags |= STARTF_USESTDHANDLES; + // Open stdout file if provided, otherwise use the null device if (stdout_file) { hStdout = CreateFile(stdout_file, GENERIC_WRITE, 0, &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); - if (hStdout == INVALID_HANDLE_VALUE) { - fprintf(stderr, "Failed to open stdout file\n"); - return; - } } else { - hStdout = CreateFile("NUL", GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); - if (hStdout == INVALID_HANDLE_VALUE) { - fprintf(stderr, "Failed to open null device for stdout\n"); - return; - } + hStdout = CreateFile("NUL", GENERIC_WRITE, 0, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + } + + if (hStdout == INVALID_HANDLE_VALUE) { + fprintf(stderr, "Failed to open stdout sink\n"); + CloseHandle(hStdin); + return; } + si.hStdOutput = hStdout; - si.dwFlags |= STARTF_USESTDHANDLES; // Open stderr file if provided, otherwise use the null device if (stderr_file) { hStderr = CreateFile(stderr_file, GENERIC_WRITE, 0, &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); - if (hStderr == INVALID_HANDLE_VALUE) { - fprintf(stderr, "Failed to open stderr file\n"); - return; - } } else { - hStderr = CreateFile("NUL", GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); - if (hStderr == INVALID_HANDLE_VALUE) { - fprintf(stderr, "Failed to open null device for stderr\n"); - return; - } + hStderr = CreateFile("NUL", GENERIC_WRITE, 0, &sa, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + } + + if (hStderr == INVALID_HANDLE_VALUE) { + fprintf(stderr, "Failed to open stderr sink\n"); + CloseHandle(hStdin); + CloseHandle(hStdout); + return; } + si.hStdError = hStderr; - si.dwFlags |= STARTF_USESTDHANDLES; - // Prepare the command line with redirected stdin - char* full_cmd; + // Prepare the command line size_t cmd_len = strlen(cmd); - size_t stdin_len = stdin_file ? strlen(stdin_file) : 0; - size_t full_cmd_len = cmd_len + stdin_len + 5; + size_t full_cmd_len = cmd_len + 1; full_cmd = (char*)malloc(full_cmd_len); if (!full_cmd) { fprintf(stderr, "Failed to allocate memory for full_cmd\n"); + CloseHandle(hStdin); + CloseHandle(hStdout); + CloseHandle(hStderr); return; } // Use full_cmd as needed (e.g., pass to CreateProcess) - if (stdin_file) { - snprintf(full_cmd, full_cmd_len, "%s < %s", cmd, stdin_file); - } else { - snprintf(full_cmd, full_cmd_len, "%s", cmd); - } + snprintf(full_cmd, full_cmd_len, "%s", cmd); + // Create the process BOOL success = CreateProcess( @@ -129,12 +138,16 @@ void process_create_windows(const char* cmd, const char* stdin_stream, if (!success) { fprintf(stderr, "CreateProcess failed (%lu).\n", GetLastError()); + CloseHandle(hStdin); + CloseHandle(hStdout); + CloseHandle(hStderr); return; } - // Close unneeded handles - if (hStdout) CloseHandle(hStdout); - if (hStderr) CloseHandle(hStderr); + // Close unneeded handles (the child has its own duplicates now) + CloseHandle(hStdin); + CloseHandle(hStdout); + CloseHandle(hStderr); // Return the process handle for status queries CloseHandle(pi.hThread); // Close the thread handle diff --git a/test/system/test_subprocess.f90 b/test/system/test_subprocess.f90 index 248e9bb8e..d025cf5b2 100644 --- a/test/system/test_subprocess.f90 +++ b/test/system/test_subprocess.f90 @@ -15,7 +15,8 @@ subroutine collect_suite(testsuite) new_unittest('test_run_synchronous', test_run_synchronous), & new_unittest('test_run_asynchronous', test_run_asynchronous), & new_unittest('test_process_kill', test_process_kill), & - new_unittest('test_process_state', test_process_state) & + new_unittest('test_process_state', test_process_state), & + new_unittest('test_input_redirection', test_input_redirection) & ] end subroutine collect_suite @@ -116,6 +117,34 @@ subroutine test_process_state(error) if (allocated(error)) return end subroutine test_process_state + !> Test input redirection + subroutine test_input_redirection(error) + type(error_type), allocatable, intent(out) :: error + type(process_type) :: process + character(len=*), parameter :: input_string = "Hello Stdin" + + if (is_windows()) then + ! findstr "^" echoes input lines. + ! Note: We need complex quoting because of how arguments are parsed. + ! Actually, sticking to something simpler if possible. + ! "more" implies paging which might hang. "sort" is usually safe. + process = run("sort", stdin=input_string, want_stdout=.true.) + else + process = run("cat", stdin=input_string, want_stdout=.true.) + endif + + call check(error, process%completed, "Process did not complete") + if (allocated(error)) return + + call check(error, process%exit_code == 0, "Process failed with non-zero exit code") + if (allocated(error)) return + + ! Check if output matches input (sort of "Hello Stdin" is "Hello Stdin") + call check(error, index(process%stdout, input_string) > 0, & + "Output <"//trim(process%stdout)//"> should contain <"//input_string//">") + + end subroutine test_input_redirection + end module test_subprocess program tester