From 60aeceb8c41fffee197d43e7eae2b46d9e3fcc74 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 28 Nov 2012 14:33:35 -0800 Subject: [PATCH] * callproc.c (Fcall_process): Fix vfork portability problems. Do not assume that fd[0], count, filefd, and save_environ survive vfork. Fix bug whereby wrong errno value could be reported for pipe failure. Some minor cleanups, too, as follows. Move buf and bufsize to the context where they're needed. Change new_argv to be of type char **, as this is more convenient and avoids casts. (CALLPROC_BUFFER_SIZE_MIN, CALLPROC_BUFFER_SIZE_MAX): Now local constants, not macros. --- src/ChangeLog | 11 +++++++ src/callproc.c | 86 ++++++++++++++++++++++++++++---------------------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index efa404afc2f..da15a612a19 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,14 @@ +2012-11-28 Paul Eggert + + * callproc.c (Fcall_process): Fix vfork portability problems. + Do not assume that fd[0], count, filefd, and save_environ survive + vfork. Fix bug whereby wrong errno value could be reported for + pipe failure. Some minor cleanups, too, as follows. Move buf and + bufsize to the context where they're needed. Change new_argv to + be of type char **, as this is more convenient and avoids casts. + (CALLPROC_BUFFER_SIZE_MIN, CALLPROC_BUFFER_SIZE_MAX): + Now local constants, not macros. + 2012-11-18 Kenichi Handa * font.c (font_unparse_xlfd): Fix previous change. Keep "const" diff --git a/src/callproc.c b/src/callproc.c index c9a504746b3..bba1c043b4c 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -183,16 +183,11 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) { Lisp_Object infile, buffer, current_dir, path, cleanup_info_tail; bool display_p; - int fd[2]; - int filefd; -#define CALLPROC_BUFFER_SIZE_MIN (16 * 1024) -#define CALLPROC_BUFFER_SIZE_MAX (4 * CALLPROC_BUFFER_SIZE_MIN) - char buf[CALLPROC_BUFFER_SIZE_MAX]; - int bufsize = CALLPROC_BUFFER_SIZE_MIN; + int fd0, fd1, filefd; ptrdiff_t count = SPECPDL_INDEX (); USE_SAFE_ALLOCA; - register const unsigned char **new_argv; + char **new_argv; /* File to use for stderr in the child. t means use same as standard output. */ Lisp_Object error_file; @@ -432,12 +427,12 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) } UNGCPRO; for (i = 4; i < nargs; i++) - new_argv[i - 3] = SDATA (args[i]); + new_argv[i - 3] = SSDATA (args[i]); new_argv[i - 3] = 0; } else new_argv[1] = 0; - new_argv[0] = SDATA (path); + new_argv[0] = SSDATA (path); #ifdef MSDOS /* MW, July 1993 */ @@ -466,29 +461,35 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) } else outfilefd = fd_output; - fd[0] = filefd; - fd[1] = outfilefd; + fd0 = filefd; + fd1 = outfilefd; #endif /* MSDOS */ if (INTEGERP (buffer)) - fd[1] = emacs_open (NULL_DEVICE, O_WRONLY, 0), fd[0] = -1; + { + fd0 = -1; + fd1 = emacs_open (NULL_DEVICE, O_WRONLY, 0); + } else { #ifndef MSDOS - errno = 0; + int fd[2]; if (pipe (fd) == -1) { + int pipe_errno = errno; emacs_close (filefd); + errno = pipe_errno; report_file_error ("Creating process pipe", Qnil); } + fd0 = fd[0]; + fd1 = fd[1]; #endif } { /* child_setup must clobber environ in systems with true vfork. Protect it from permanent change. */ - register char **save_environ = environ; - register int fd1 = fd[1]; + char **save_environ = environ; int fd_error = fd1; if (fd_output >= 0) @@ -520,8 +521,8 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) if (fd_error < 0) { emacs_close (filefd); - if (fd[0] != filefd) - emacs_close (fd[0]); + if (fd0 != filefd) + emacs_close (fd0); if (fd1 >= 0) emacs_close (fd1); #ifdef MSDOS @@ -538,8 +539,7 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) /* Note that on MSDOS `child_setup' actually returns the child process exit status, not its PID, so we assign it to `synch_process_retcode' below. */ - pid = child_setup (filefd, outfilefd, fd_error, (char **) new_argv, - 0, current_dir); + pid = child_setup (filefd, outfilefd, fd_error, new_argv, 0, current_dir); /* Record that the synchronous process exited and note its termination status. */ @@ -559,8 +559,8 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) { /* Since CRLF is converted to LF within `decode_coding', we can always open a file with binary mode. */ - fd[0] = emacs_open (tempfile, O_RDONLY | O_BINARY, 0); - if (fd[0] < 0) + fd0 = emacs_open (tempfile, O_RDONLY | O_BINARY, 0); + if (fd0 < 0) { unlink (tempfile); emacs_close (filefd); @@ -569,11 +569,10 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) } } else - fd[0] = -1; /* We are not going to read from tempfile. */ + fd0 = -1; /* We are not going to read from tempfile. */ #else /* not MSDOS */ #ifdef WINDOWSNT - pid = child_setup (filefd, fd1, fd_error, (char **) new_argv, - 0, current_dir); + pid = child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir); #else /* not WINDOWSNT */ block_input (); @@ -586,11 +585,15 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) bool volatile display_p_volatile = display_p; bool volatile output_to_buffer_volatile = output_to_buffer; bool volatile sa_must_free_volatile = sa_must_free; + int volatile fd0_volatile = fd0; int volatile fd1_volatile = fd1; int volatile fd_error_volatile = fd_error; int volatile fd_output_volatile = fd_output; + int volatile filefd_volatile = filefd; + ptrdiff_t volatile count_volatile = count; ptrdiff_t volatile sa_count_volatile = sa_count; - unsigned char const **volatile new_argv_volatile = new_argv; + char **volatile new_argv_volatile = new_argv; + char **volatile new_save_environ = save_environ; pid = vfork (); @@ -598,27 +601,30 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) coding_systems = coding_systems_volatile; current_dir = current_dir_volatile; display_p = display_p_volatile; + output_to_buffer = output_to_buffer_volatile; + sa_must_free = sa_must_free_volatile; + fd0 = fd0_volatile; fd1 = fd1_volatile; fd_error = fd_error_volatile; fd_output = fd_output_volatile; - output_to_buffer = output_to_buffer_volatile; - sa_must_free = sa_must_free_volatile; + filefd = filefd_volatile; + count = count_volatile; sa_count = sa_count_volatile; new_argv = new_argv_volatile; + save_environ = new_save_environ; } if (pid == 0) { - if (fd[0] >= 0) - emacs_close (fd[0]); + if (fd0 >= 0) + emacs_close (fd0); setsid (); /* Emacs ignores SIGPIPE, but the child should not. */ signal (SIGPIPE, SIG_DFL); - child_setup (filefd, fd1, fd_error, (char **) new_argv, - 0, current_dir); + child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir); } unblock_input (); @@ -632,7 +638,7 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) environ = save_environ; - /* Close most of our fd's, but not fd[0] + /* Close most of our file descriptors, but not fd0 since we will use that to read input from. */ emacs_close (filefd); if (fd_output >= 0) @@ -643,15 +649,15 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) if (pid < 0) { - if (fd[0] >= 0) - emacs_close (fd[0]); + if (fd0 >= 0) + emacs_close (fd0); report_file_error ("Doing vfork", Qnil); } if (INTEGERP (buffer)) { - if (fd[0] >= 0) - emacs_close (fd[0]); + if (fd0 >= 0) + emacs_close (fd0); return Qnil; } @@ -666,7 +672,7 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) #endif /* not MSDOS */ record_unwind_protect (call_process_cleanup, Fcons (Fcurrent_buffer (), - Fcons (INTEGER_TO_CONS (fd[0]), + Fcons (INTEGER_TO_CONS (fd0), cleanup_info_tail))); if (BUFFERP (buffer)) @@ -723,6 +729,10 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) if (output_to_buffer) { + enum { CALLPROC_BUFFER_SIZE_MIN = 16 * 1024 }; + enum { CALLPROC_BUFFER_SIZE_MAX = 4 * CALLPROC_BUFFER_SIZE_MIN }; + char buf[CALLPROC_BUFFER_SIZE_MAX]; + int bufsize = CALLPROC_BUFFER_SIZE_MIN; int nread; bool first = 1; EMACS_INT total_read = 0; @@ -739,7 +749,7 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) nread = carryover; while (nread < bufsize - 1024) { - int this_read = emacs_read (fd[0], buf + nread, + int this_read = emacs_read (fd0, buf + nread, bufsize - nread); if (this_read < 0) -- 2.39.5