diff options
| -rw-r--r-- | src/ChangeLog | 11 | ||||
| -rw-r--r-- | 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 <eggert@cs.ucla.edu> + + * 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 <handa@gnu.org> * 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) |