From 83811e3f1f0c615b2b63bafdb49a35a0fc198088 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 28 Sep 2015 18:10:21 +0200 Subject: [PATCH] common: Change calling convention for gnupg_spawn_process. * common/exechelp.h (GNUPG_SPAWN_NONBLOCK): New. (GNUPG_SPAWN_RUN_ASFW, GNUPG_SPAWN_DETACHED): Macro to replace the numbers. * common/exechelp.h (gnupg_spawn_process): Change function to not take an optional stream for stdin but to return one. * common/exechelp-posix.c (gnupg_spawn_process): Implement change. (create_pipe_and_estream): Add args outbound and nonblock. * common/exechelp-w32.c (gnupg_spawn_process): Implement change. -- In 2.1 this function is only used at one place and the stdin parameter is not used. Thus this change is trivial for the callers but along with estream's new es_poll it is overall simpler to use. Note that the Windows version has not been tested. Signed-off-by: Werner Koch --- common/exechelp-posix.c | 70 ++++++++++++++++++++++++++----------- common/exechelp-w32.c | 76 ++++++++++++++++++++++++++--------------- common/exechelp-w32ce.c | 2 +- common/exechelp.h | 40 +++++++++++++++------- 4 files changed, 128 insertions(+), 60 deletions(-) diff --git a/common/exechelp-posix.c b/common/exechelp-posix.c index 8a2b3b9c2..2bf2592e0 100644 --- a/common/exechelp-posix.c +++ b/common/exechelp-posix.c @@ -313,6 +313,7 @@ gnupg_create_outbound_pipe (int filedes[2]) static gpg_error_t create_pipe_and_estream (int filedes[2], estream_t *r_fp, + int outbound, int nonblock, gpg_err_source_t errsource) { gpg_error_t err; @@ -326,7 +327,10 @@ create_pipe_and_estream (int filedes[2], estream_t *r_fp, return err; } - *r_fp = es_fdopen (filedes[0], "r"); + if (outbound) + *r_fp = es_fdopen (filedes[0], nonblock? "r,nonblock" : "r"); + else + *r_fp = es_fdopen (filedes[1], nonblock? "w,nonblock" : "w"); if (!*r_fp) { err = gpg_err_make (errsource, gpg_err_code_from_syserror ()); @@ -347,53 +351,70 @@ gpg_error_t gnupg_spawn_process (const char *pgmname, const char *argv[], gpg_err_source_t errsource, void (*preexec)(void), unsigned int flags, - estream_t infp, + estream_t *r_infp, estream_t *r_outfp, estream_t *r_errfp, pid_t *pid) { gpg_error_t err; - int infd = -1; + int inpipe[2] = {-1, -1}; int outpipe[2] = {-1, -1}; int errpipe[2] = {-1, -1}; + estream_t infp = NULL; estream_t outfp = NULL; estream_t errfp = NULL; + int nonblock = !!(flags & GNUPG_SPAWN_NONBLOCK); - (void)flags; /* Currently not used. */ - + if (r_infp) + *r_infp = NULL; if (r_outfp) *r_outfp = NULL; if (r_errfp) *r_errfp = NULL; *pid = (pid_t)(-1); /* Always required. */ - if (infp) + if (r_infp) { - es_fflush (infp); - es_rewind (infp); - infd = es_fileno (infp); - if (infd == -1) - return gpg_err_make (errsource, GPG_ERR_INV_VALUE); - } - - if (r_outfp) - { - err = create_pipe_and_estream (outpipe, &outfp, errsource); + err = create_pipe_and_estream (inpipe, &infp, 0, nonblock, errsource); if (err) return err; } - if (r_errfp) + if (r_outfp) { - err = create_pipe_and_estream (errpipe, &errfp, errsource); + err = create_pipe_and_estream (outpipe, &outfp, 1, nonblock, errsource); if (err) { + if (infp) + es_fclose (infp); + else if (inpipe[1] != -1) + close (inpipe[1]); + if (inpipe[0] != -1) + close (inpipe[0]); + + return err; + } + } + + if (r_errfp) + { + err = create_pipe_and_estream (errpipe, &errfp, 1, nonblock, errsource); + if (err) + { + if (infp) + es_fclose (infp); + else if (inpipe[1] != -1) + close (inpipe[1]); + if (inpipe[0] != -1) + close (inpipe[0]); + if (outfp) es_fclose (outfp); else if (outpipe[0] != -1) close (outpipe[0]); if (outpipe[1] != -1) close (outpipe[1]); + return err; } } @@ -405,6 +426,13 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], err = gpg_err_make (errsource, gpg_err_code_from_syserror ()); log_error (_("error forking process: %s\n"), gpg_strerror (err)); + if (infp) + es_fclose (infp); + else if (inpipe[1] != -1) + close (inpipe[1]); + if (inpipe[0] != -1) + close (inpipe[0]); + if (outfp) es_fclose (outfp); else if (outpipe[0] != -1) @@ -427,16 +455,20 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], gcry_control (GCRYCTL_TERM_SECMEM); es_fclose (outfp); es_fclose (errfp); - do_exec (pgmname, argv, infd, outpipe[1], errpipe[1], preexec); + do_exec (pgmname, argv, inpipe[0], outpipe[1], errpipe[1], preexec); /*NOTREACHED*/ } /* This is the parent. */ + if (inpipe[0] != -1) + close (inpipe[0]); if (outpipe[1] != -1) close (outpipe[1]); if (errpipe[1] != -1) close (errpipe[1]); + if (r_infp) + *r_infp = infp; if (r_outfp) *r_outfp = outfp; if (r_errfp) diff --git a/common/exechelp-w32.c b/common/exechelp-w32.c index 05e9e1000..ba3b357e9 100644 --- a/common/exechelp-w32.c +++ b/common/exechelp-w32.c @@ -346,7 +346,7 @@ gpg_error_t gnupg_spawn_process (const char *pgmname, const char *argv[], gpg_err_source_t errsource, void (*preexec)(void), unsigned int flags, - estream_t infp, + estream_t *r_infp, estream_t *r_outfp, estream_t *r_errfp, pid_t *pid) @@ -363,9 +363,10 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], STARTUPINFO si; int cr_flags; char *cmdline; - HANDLE inhandle = INVALID_HANDLE_VALUE; + HANDLE inpipe[2] = {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE}; HANDLE outpipe[2] = {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE}; HANDLE errpipe[2] = {INVALID_HANDLE_VALUE, INVALID_HANDLE_VALUE}; + estream_t infp = NULL; estream_t outfp = NULL; estream_t errfp = NULL; HANDLE nullhd[3] = {INVALID_HANDLE_VALUE, @@ -374,6 +375,8 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], int i; es_syshd_t syshd; + if (r_infp) + *r_infp = NULL; if (r_outfp) *r_outfp = NULL; if (r_errfp) @@ -382,29 +385,26 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], if (infp) { - es_fflush (infp); - es_rewind (infp); - es_syshd (infp, &syshd); - switch (syshd.type) + if (create_inheritable_pipe (inpipe, 0)) { - case ES_SYSHD_FD: - inhandle = (HANDLE)_get_osfhandle (syshd.u.fd); - break; - case ES_SYSHD_SOCK: - inhandle = (HANDLE)_get_osfhandle (syshd.u.sock); - break; - case ES_SYSHD_HANDLE: - inhandle = syshd.u.handle; - break; - default: - inhandle = INVALID_HANDLE_VALUE; - break; + err = gpg_err_make (errsource, GPG_ERR_GENERAL); + log_error (_("error creating a pipe: %s\n"), gpg_strerror (err)); + return err; + } + + syshd.type = ES_SYSHD_HANDLE; + syshd.u.handle = inpipe[1]; + infp = es_sysopen (&syshd, "w"); + if (!infp) + { + err = gpg_err_make (errsource, gpg_err_code_from_syserror ()); + log_error (_("error creating a stream for a pipe: %s\n"), + gpg_strerror (err)); + CloseHandle (inpipe[0]); + CloseHandle (inpipe[1]); + inpipe[0] = inpipe[1] = INVALID_HANDLE_VALUE; + return err; } - if (inhandle == INVALID_HANDLE_VALUE) - return gpg_err_make (errsource, GPG_ERR_INV_VALUE); - /* FIXME: In case we can't get a system handle (e.g. due to - es_fopencookie we should create a piper and a feeder - thread. */ } if (r_outfp) @@ -427,6 +427,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], CloseHandle (outpipe[0]); CloseHandle (outpipe[1]); outpipe[0] = outpipe[1] = INVALID_HANDLE_VALUE; + if (infp) + es_fclose (infp); + else if (inpipe[1] != INVALID_HANDLE_VALUE) + CloseHandle (outpipe[1]); + if (inpipe[0] != INVALID_HANDLE_VALUE) + CloseHandle (inpipe[0]); return err; } } @@ -457,6 +463,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], CloseHandle (outpipe[0]); if (outpipe[1] != INVALID_HANDLE_VALUE) CloseHandle (outpipe[1]); + if (infp) + es_fclose (infp); + else if (inpipe[1] != INVALID_HANDLE_VALUE) + CloseHandle (outpipe[1]); + if (inpipe[0] != INVALID_HANDLE_VALUE) + CloseHandle (inpipe[0]); return err; } } @@ -471,7 +483,7 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], if (err) return err; - if (inhandle != INVALID_HANDLE_VALUE) + if (inpipe[0] != INVALID_HANDLE_VALUE) nullhd[0] = w32_open_null (0); if (outpipe[1] != INVALID_HANDLE_VALUE) nullhd[1] = w32_open_null (0); @@ -486,12 +498,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], si.cb = sizeof (si); si.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW; si.wShowWindow = DEBUG_W32_SPAWN? SW_SHOW : SW_MINIMIZE; - si.hStdInput = inhandle == INVALID_HANDLE_VALUE? nullhd[0] : inhandle; + si.hStdInput = inpipe[0] == INVALID_HANDLE_VALUE? nullhd[0] : inpipe[0]; si.hStdOutput = outpipe[1] == INVALID_HANDLE_VALUE? nullhd[1] : outpipe[1]; si.hStdError = errpipe[1] == INVALID_HANDLE_VALUE? nullhd[2] : errpipe[1]; cr_flags = (CREATE_DEFAULT_ERROR_MODE - | ((flags & 128)? DETACHED_PROCESS : 0) + | ((flags & GNUPG_SPAWN_DETACHED)? DETACHED_PROCESS : 0) | GetPriorityClass (GetCurrentProcess ()) | CREATE_SUSPENDED); /* log_debug ("CreateProcess, path='%s' cmdline='%s'\n", pgmname, cmdline); */ @@ -509,6 +521,12 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], { log_error ("CreateProcess failed: %s\n", w32_strerror (-1)); xfree (cmdline); + if (infp) + es_fclose (infp); + else if (inpipe[1] != INVALID_HANDLE_VALUE) + CloseHandle (outpipe[1]); + if (inpipe[0] != INVALID_HANDLE_VALUE) + CloseHandle (inpipe[0]); if (outfp) es_fclose (outfp); else if (outpipe[0] != INVALID_HANDLE_VALUE) @@ -532,6 +550,8 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], CloseHandle (nullhd[i]); /* Close the inherited ends of the pipes. */ + if (inpipe[0] != INVALID_HANDLE_VALUE) + CloseHandle (inpipe[0]); if (outpipe[1] != INVALID_HANDLE_VALUE) CloseHandle (outpipe[1]); if (errpipe[1] != INVALID_HANDLE_VALUE) @@ -546,13 +566,15 @@ gnupg_spawn_process (const char *pgmname, const char *argv[], /* Fixme: For unknown reasons AllowSetForegroundWindow returns an invalid argument error if we pass it the correct processID. As a workaround we use -1 (ASFW_ANY). */ - if ( (flags & 64) ) + if ((flags & GNUPG_SPAWN_RUN_ASFW)) gnupg_allow_set_foregound_window ((pid_t)(-1)/*pi.dwProcessId*/); /* Process has been created suspended; resume it now. */ ResumeThread (pi.hThread); CloseHandle (pi.hThread); + if (r_infp) + *r_infp = infp; if (r_outfp) *r_outfp = outfp; if (r_errfp) diff --git a/common/exechelp-w32ce.c b/common/exechelp-w32ce.c index cca55c8ce..975386a13 100644 --- a/common/exechelp-w32ce.c +++ b/common/exechelp-w32ce.c @@ -502,7 +502,7 @@ gpg_error_t gnupg_spawn_process (const char *pgmname, const char *argv[], gpg_err_source_t errsource, void (*preexec)(void), unsigned int flags, - estream_t infp, + estream_t *r_infp, estream_t *r_outfp, estream_t *r_errfp, pid_t *pid) diff --git a/common/exechelp.h b/common/exechelp.h index a9a9ca304..a146d893a 100644 --- a/common/exechelp.h +++ b/common/exechelp.h @@ -59,17 +59,22 @@ gpg_error_t gnupg_create_inbound_pipe (int filedes[2]); inheritable. */ gpg_error_t gnupg_create_outbound_pipe (int filedes[2]); +#define GNUPG_SPAWN_NONBLOCK 16 +#define GNUPG_SPAWN_RUN_ASFW 64 +#define GNUPG_SPAWN_DETACHED 128 -/* Fork and exec the PGMNAME. If INFP is NULL connect /dev/null to - stdin of the new process; if it is not NULL connect the file - descriptor retrieved from INFP to stdin. If R_OUTFP is NULL - connect stdout of the new process to /dev/null; if it is not NULL - store the address of a pointer to a new estream there. If R_ERRFP - is NULL connect stderr of the new process to /dev/null; if it is - not NULL store the address of a pointer to a new estream there. On - success the pid of the new process is stored at PID. On error -1 - is stored at PID and if R_OUTFP or R_ERRFP are not NULL, NULL is - stored there. + +/* Fork and exec the program PGMNAME. + + If R_INFP is NULL connect stdin of the new process to /dev/null; if + it is not NULL store the address of a pointer to a new estream + there. If R_OUTFP is NULL connect stdout of the new process to + /dev/null; if it is not NULL store the address of a pointer to a + new estream there. If R_ERRFP is NULL connect stderr of the new + process to /dev/null; if it is not NULL store the address of a + pointer to a new estream there. On success the pid of the new + process is stored at PID. On error -1 is stored at PID and if + R_OUTFP or R_ERRFP are not NULL, NULL is stored there. The arguments for the process are expected in the NULL terminated array ARGV. The program name itself should not be included there. @@ -81,12 +86,21 @@ gpg_error_t gnupg_create_outbound_pipe (int filedes[2]); FLAGS is a bit vector: - Bit 7: If set the process will be started as a background process. + GNUPG_SPAWN_NONBLOCK + If set the two output streams are created in non-blocking + mode and the input stream is switched to non-blocking mode. + This is merely a convenience feature because the caller + could do the same with gpgrt_set_nonblock. Does not yet + work for Windows. + + GNUPG_SPAWN_DETACHED + If set the process will be started as a background process. This flag is only useful under W32 (but not W32CE) systems, so that no new console is created and pops up a console window when starting the server. Does not work on W32CE. - Bit 6: On W32 (but not on W32CE) run AllowSetForegroundWindow for + GNUPG_SPAWN_RUN_ASFW + On W32 (but not on W32CE) run AllowSetForegroundWindow for the child. Note that due to unknown problems this actually allows SetForegroundWindow for all childs of this process. @@ -95,7 +109,7 @@ gpg_error_t gnupg_spawn_process (const char *pgmname, const char *argv[], gpg_err_source_t errsource, void (*preexec)(void), unsigned int flags, - estream_t infp, + estream_t *r_infp, estream_t *r_outfp, estream_t *r_errfp, pid_t *pid);