From eb4f9669716367912244182dd0eb788759f800ec Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 28 Jan 2012 16:50:21 +0000 Subject: Fix failures on MS-Windows when Make's standard handles are invalid. This can happen when Make is invoked from a GUI application. * w32/subproc/sub_proc.c (process_init_fd): Don't dereference pproc if it is a NULL pointer. (process_begin, process_cleanup): Don't try to close pipe handles whose value is INVALID_HANDLE_VALUE. (process_easy): Initialize hIn, hOut, and hErr to INVALID_HANDLE_VALUE. If DuplicateHandle fails with ERROR_INVALID_HANDLE, duplicate a handle for the null device instead of STD_INPUT_HANDLE, STD_OUTPUT_HANDLE or STD_ERROR_HANDLE. Don't try to close pipe handles whose value is INVALID_HANDLE_VALUE. * function.c (windows32_openpipe): Initialize hIn and hErr to INVALID_HANDLE_VALUE. If DuplicateHandle fails with ERROR_INVALID_HANDLE, duplicate a handle for the null device instead of STD_INPUT_HANDLE or STD_ERROR_HANDLE. Fix indentation. Don't try to close handles whose value is INVALID_HANDLE_VALUE. --- ChangeLog | 22 +++++++ function.c | 73 ++++++++++++++++------ w32/subproc/sub_proc.c | 165 +++++++++++++++++++++++++++++++++---------------- 3 files changed, 189 insertions(+), 71 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7dbfdb2..ca06ec7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2012-01-28 Eli Zaretskii + + Fix failures on MS-Windows when Make's standard handles are invalid. + This can happen when Make is invoked from a GUI application. + + * w32/subproc/sub_proc.c (process_init_fd): Don't dereference + pproc if it is a NULL pointer. + (process_begin, process_cleanup): Don't try to close pipe handles + whose value is INVALID_HANDLE_VALUE. + (process_easy): Initialize hIn, hOut, and hErr to + INVALID_HANDLE_VALUE. If DuplicateHandle fails with + ERROR_INVALID_HANDLE, duplicate a handle for the null device + instead of STD_INPUT_HANDLE, STD_OUTPUT_HANDLE or + STD_ERROR_HANDLE. Don't try to close pipe handles whose value is + INVALID_HANDLE_VALUE. + + * function.c (windows32_openpipe): Initialize hIn and hErr to + INVALID_HANDLE_VALUE. If DuplicateHandle fails with + ERROR_INVALID_HANDLE, duplicate a handle for the null device + instead of STD_INPUT_HANDLE or STD_ERROR_HANDLE. Fix indentation. + Don't try to close handles whose value is INVALID_HANDLE_VALUE. + 2012-01-25 Eli Zaretskii * function.c (define_new_function): Fix format strings in calls to diff --git a/function.c b/function.c index 52235db..29b106f 100644 --- a/function.c +++ b/function.c @@ -1434,37 +1434,70 @@ void windows32_openpipe (int *pipedes, pid_t *pid_p, char **command_argv, char **envp) { SECURITY_ATTRIBUTES saAttr; - HANDLE hIn; - HANDLE hErr; + HANDLE hIn = INVALID_HANDLE_VALUE; + HANDLE hErr = INVALID_HANDLE_VALUE; HANDLE hChildOutRd; HANDLE hChildOutWr; - HANDLE hProcess; - + HANDLE hProcess, tmpIn, tmpErr; + DWORD e; saAttr.nLength = sizeof (SECURITY_ATTRIBUTES); saAttr.bInheritHandle = TRUE; saAttr.lpSecurityDescriptor = NULL; + /* Standard handles returned by GetStdHandle can be NULL or + INVALID_HANDLE_VALUE if the parent process closed them. If that + happens, we open the null device and pass its handle to + process_begin below as the corresponding handle to inherit. */ + tmpIn = GetStdHandle(STD_INPUT_HANDLE); if (DuplicateHandle (GetCurrentProcess(), - GetStdHandle(STD_INPUT_HANDLE), + tmpIn, GetCurrentProcess(), &hIn, 0, TRUE, DUPLICATE_SAME_ACCESS) == FALSE) { - fatal (NILF, _("windows32_openpipe(): DuplicateHandle(In) failed (e=%ld)\n"), - GetLastError()); - + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) { + tmpIn = CreateFile("NUL", GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (tmpIn != INVALID_HANDLE_VALUE + && DuplicateHandle(GetCurrentProcess(), + tmpIn, + GetCurrentProcess(), + &hIn, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) + CloseHandle(tmpIn); + } + if (hIn == INVALID_HANDLE_VALUE) + fatal (NILF, _("windows32_openpipe: DuplicateHandle(In) failed (e=%ld)\n"), e); } + tmpErr = GetStdHandle(STD_ERROR_HANDLE); if (DuplicateHandle(GetCurrentProcess(), - GetStdHandle(STD_ERROR_HANDLE), + tmpErr, GetCurrentProcess(), &hErr, 0, TRUE, DUPLICATE_SAME_ACCESS) == FALSE) { - fatal (NILF, _("windows32_open_pipe(): DuplicateHandle(Err) failed (e=%ld)\n"), - GetLastError()); + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) { + tmpErr = CreateFile("NUL", GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (tmpErr != INVALID_HANDLE_VALUE + && DuplicateHandle(GetCurrentProcess(), + tmpErr, + GetCurrentProcess(), + &hErr, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) + CloseHandle(tmpErr); + } + if (hErr == INVALID_HANDLE_VALUE) + fatal (NILF, _("windows32_openpipe: DuplicateHandle(Err) failed (e=%ld)\n"), e); } if (!CreatePipe(&hChildOutRd, &hChildOutWr, &saAttr, 0)) @@ -1483,23 +1516,25 @@ windows32_openpipe (int *pipedes, pid_t *pid_p, char **command_argv, char **envp if (!process_begin(hProcess, command_argv, envp, command_argv[0], NULL)) { /* register process for wait */ - process_register(hProcess); + process_register(hProcess); /* set the pid for returning to caller */ - *pid_p = (pid_t) hProcess; + *pid_p = (pid_t) hProcess; - /* set up to read data from child */ - pipedes[0] = _open_osfhandle((intptr_t) hChildOutRd, O_RDONLY); + /* set up to read data from child */ + pipedes[0] = _open_osfhandle((intptr_t) hChildOutRd, O_RDONLY); - /* this will be closed almost right away */ - pipedes[1] = _open_osfhandle((intptr_t) hChildOutWr, O_APPEND); + /* this will be closed almost right away */ + pipedes[1] = _open_osfhandle((intptr_t) hChildOutWr, O_APPEND); } else { /* reap/cleanup the failed process */ process_cleanup(hProcess); /* close handles which were duplicated, they weren't used */ - CloseHandle(hIn); - CloseHandle(hErr); + if (hIn != INVALID_HANDLE_VALUE) + CloseHandle(hIn); + if (hErr != INVALID_HANDLE_VALUE) + CloseHandle(hErr); /* close pipe handles, they won't be used */ CloseHandle(hChildOutRd); diff --git a/w32/subproc/sub_proc.c b/w32/subproc/sub_proc.c index be5f1a2..2b1b29c 100644 --- a/w32/subproc/sub_proc.c +++ b/w32/subproc/sub_proc.c @@ -462,17 +462,19 @@ process_init_fd(HANDLE stdinh, HANDLE stdouth, HANDLE stderrh) sub_process *pproc; pproc = malloc(sizeof(*pproc)); - memset(pproc, 0, sizeof(*pproc)); + if (pproc) { + memset(pproc, 0, sizeof(*pproc)); - /* - * Just pass the provided file handles to the 'child side' of the - * pipe, bypassing pipes altogether. - */ - pproc->sv_stdin[1] = (intptr_t) stdinh; - pproc->sv_stdout[1] = (intptr_t) stdouth; - pproc->sv_stderr[1] = (intptr_t) stderrh; + /* + * Just pass the provided file handles to the 'child + * side' of the pipe, bypassing pipes altogether. + */ + pproc->sv_stdin[1] = (intptr_t) stdinh; + pproc->sv_stdout[1] = (intptr_t) stdouth; + pproc->sv_stderr[1] = (intptr_t) stderrh; - pproc->last_err = pproc->lerrno = 0; + pproc->last_err = pproc->lerrno = 0; + } return((HANDLE)pproc); } @@ -711,9 +713,12 @@ process_begin( CloseHandle(procInfo.hThread); /* Close the halves of the pipes we don't need */ - CloseHandle((HANDLE)pproc->sv_stdin[1]); - CloseHandle((HANDLE)pproc->sv_stdout[1]); - CloseHandle((HANDLE)pproc->sv_stderr[1]); + if ((HANDLE)pproc->sv_stdin[1] != INVALID_HANDLE_VALUE) + CloseHandle((HANDLE)pproc->sv_stdin[1]); + if ((HANDLE)pproc->sv_stdout[1] != INVALID_HANDLE_VALUE) + CloseHandle((HANDLE)pproc->sv_stdout[1]); + if ((HANDLE)pproc->sv_stderr[1] != INVALID_HANDLE_VALUE) + CloseHandle((HANDLE)pproc->sv_stderr[1]); pproc->sv_stdin[1] = 0; pproc->sv_stdout[1] = 0; pproc->sv_stderr[1] = 0; @@ -1064,11 +1069,14 @@ process_cleanup( if (pproc->using_pipes) { for (i= 0; i <= 1; i++) { - if ((HANDLE)pproc->sv_stdin[i]) + if ((HANDLE)pproc->sv_stdin[i] + && (HANDLE)pproc->sv_stdin[i] != INVALID_HANDLE_VALUE) CloseHandle((HANDLE)pproc->sv_stdin[i]); - if ((HANDLE)pproc->sv_stdout[i]) + if ((HANDLE)pproc->sv_stdout[i] + && (HANDLE)pproc->sv_stdout[i] != INVALID_HANDLE_VALUE) CloseHandle((HANDLE)pproc->sv_stdout[i]); - if ((HANDLE)pproc->sv_stderr[i]) + if ((HANDLE)pproc->sv_stderr[i] + && (HANDLE)pproc->sv_stderr[i] != INVALID_HANDLE_VALUE) CloseHandle((HANDLE)pproc->sv_stderr[i]); } } @@ -1333,50 +1341,100 @@ process_easy( char **argv, char **envp) { - HANDLE hIn; - HANDLE hOut; - HANDLE hErr; - HANDLE hProcess; + HANDLE hIn = INVALID_HANDLE_VALUE; + HANDLE hOut = INVALID_HANDLE_VALUE; + HANDLE hErr = INVALID_HANDLE_VALUE; + HANDLE hProcess, tmpIn, tmpOut, tmpErr; + DWORD e; if (proc_index >= MAXIMUM_WAIT_OBJECTS) { DB (DB_JOBS, ("process_easy: All process slots used up\n")); return INVALID_HANDLE_VALUE; } + /* Standard handles returned by GetStdHandle can be NULL or + INVALID_HANDLE_VALUE if the parent process closed them. If that + happens, we open the null device and pass its handle to + CreateProcess as the corresponding handle to inherit. */ + tmpIn = GetStdHandle(STD_INPUT_HANDLE); if (DuplicateHandle(GetCurrentProcess(), - GetStdHandle(STD_INPUT_HANDLE), - GetCurrentProcess(), - &hIn, - 0, - TRUE, - DUPLICATE_SAME_ACCESS) == FALSE) { - fprintf(stderr, - "process_easy: DuplicateHandle(In) failed (e=%ld)\n", - GetLastError()); - return INVALID_HANDLE_VALUE; + tmpIn, + GetCurrentProcess(), + &hIn, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) { + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) { + tmpIn = CreateFile("NUL", GENERIC_READ, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (tmpIn != INVALID_HANDLE_VALUE + && DuplicateHandle(GetCurrentProcess(), + tmpIn, + GetCurrentProcess(), + &hIn, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) + CloseHandle(tmpIn); + } + if (hIn == INVALID_HANDLE_VALUE) { + fprintf(stderr, "process_easy: DuplicateHandle(In) failed (e=%ld)\n", e); + return INVALID_HANDLE_VALUE; + } } + tmpOut = GetStdHandle (STD_OUTPUT_HANDLE); if (DuplicateHandle(GetCurrentProcess(), - GetStdHandle(STD_OUTPUT_HANDLE), - GetCurrentProcess(), - &hOut, - 0, - TRUE, - DUPLICATE_SAME_ACCESS) == FALSE) { - fprintf(stderr, - "process_easy: DuplicateHandle(Out) failed (e=%ld)\n", - GetLastError()); - return INVALID_HANDLE_VALUE; + tmpOut, + GetCurrentProcess(), + &hOut, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) { + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) { + tmpOut = CreateFile("NUL", GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (tmpOut != INVALID_HANDLE_VALUE + && DuplicateHandle(GetCurrentProcess(), + tmpOut, + GetCurrentProcess(), + &hOut, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) + CloseHandle(tmpOut); + } + if (hOut == INVALID_HANDLE_VALUE) { + fprintf(stderr, "process_easy: DuplicateHandle(Out) failed (e=%ld)\n", e); + return INVALID_HANDLE_VALUE; + } } + tmpErr = GetStdHandle(STD_ERROR_HANDLE); if (DuplicateHandle(GetCurrentProcess(), - GetStdHandle(STD_ERROR_HANDLE), - GetCurrentProcess(), - &hErr, - 0, - TRUE, - DUPLICATE_SAME_ACCESS) == FALSE) { - fprintf(stderr, - "process_easy: DuplicateHandle(Err) failed (e=%ld)\n", - GetLastError()); - return INVALID_HANDLE_VALUE; + tmpErr, + GetCurrentProcess(), + &hErr, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) { + if ((e = GetLastError()) == ERROR_INVALID_HANDLE) { + tmpErr = CreateFile("NUL", GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (tmpErr != INVALID_HANDLE_VALUE + && DuplicateHandle(GetCurrentProcess(), + tmpErr, + GetCurrentProcess(), + &hErr, + 0, + TRUE, + DUPLICATE_SAME_ACCESS) == FALSE) + CloseHandle(tmpErr); + } + if (hErr == INVALID_HANDLE_VALUE) { + fprintf(stderr, "process_easy: DuplicateHandle(Err) failed (e=%ld)\n", e); + return INVALID_HANDLE_VALUE; + } } hProcess = process_init_fd(hIn, hOut, hErr); @@ -1389,9 +1447,12 @@ process_easy( ((sub_process*) hProcess)->exit_code = process_last_err(hProcess); /* close up unused handles */ - CloseHandle(hIn); - CloseHandle(hOut); - CloseHandle(hErr); + if (hIn != INVALID_HANDLE_VALUE) + CloseHandle(hIn); + if (hOut != INVALID_HANDLE_VALUE) + CloseHandle(hOut); + if (hErr != INVALID_HANDLE_VALUE) + CloseHandle(hErr); } process_register(hProcess); -- cgit v1.2.3