From bafe80ce49206c61bea74b0c70080bfdc97785d1 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sun, 14 Jul 2013 19:56:17 -0700 Subject: [PATCH] * callproc.c (Fcall_process_region): Fix minor race and tune. (create_temp_file): New function, with the temp-file-creation part of the old Fcall_process_region. Use Fcopy_sequence to create the temp file name, rather than alloca + build_string, for simplicity. Don't bother to block input around the temp file creation; shouldn't be needed. Simplify use of mktemp. Use record_unwind_protect immediately after creating the temp file; this closes an unlikely race where the temp file was not removed. Use memcpy rather than an open-coded loop. (Fcall_process_region): Use the new function. If the input is empty, redirect from /dev/null rather than from a newly created empty temp file; this avoids unnecessary file system traffic. --- src/ChangeLog | 15 ++++++ src/callproc.c | 136 ++++++++++++++++++++++++++++--------------------- 2 files changed, 92 insertions(+), 59 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 329fa6ba670..cd6d188b686 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,18 @@ +2013-07-15 Paul Eggert + + * callproc.c (Fcall_process_region): Fix minor race and tune. + (create_temp_file): New function, with the temp-file-creation part + of the old Fcall_process_region. Use Fcopy_sequence to create the + temp file name, rather than alloca + build_string, for simplicity. + Don't bother to block input around the temp file creation; + shouldn't be needed. Simplify use of mktemp. Use + record_unwind_protect immediately after creating the temp file; + this closes an unlikely race where the temp file was not removed. + Use memcpy rather than an open-coded loop. + (Fcall_process_region): Use the new function. If the input is + empty, redirect from /dev/null rather than from a newly created + empty temp file; this avoids unnecessary file system traffic. + 2013-07-14 Paul Eggert * filelock.c (create_lock_file) [!HAVE_MKOSTEMP && !HAVE_MKSTEMP]: diff --git a/src/callproc.c b/src/callproc.c index cdf92422b4d..6d770f881ff 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -960,41 +960,16 @@ delete_temp_file (Lisp_Object name) return Qnil; } -DEFUN ("call-process-region", Fcall_process_region, Scall_process_region, - 3, MANY, 0, - doc: /* Send text from START to END to a synchronous process running PROGRAM. -The remaining arguments are optional. -Delete the text if fourth arg DELETE is non-nil. - -Insert output in BUFFER before point; t means current buffer; nil for - BUFFER means discard it; 0 means discard and don't wait; and `(:file - FILE)', where FILE is a file name string, means that it should be - written to that file (if the file already exists it is overwritten). -BUFFER can also have the form (REAL-BUFFER STDERR-FILE); in that case, -REAL-BUFFER says what to do with standard output, as above, -while STDERR-FILE says what to do with standard error in the child. -STDERR-FILE may be nil (discard standard error output), -t (mix it with ordinary output), or a file name string. - -Sixth arg DISPLAY non-nil means redisplay buffer as output is inserted. -Remaining args are passed to PROGRAM at startup as command args. +/* Create a temporary file suitable for storing the input data of + call-process-region. NARGS and ARGS are the same as for + call-process-region. */ -If BUFFER is 0, `call-process-region' returns immediately with value nil. -Otherwise it waits for PROGRAM to terminate -and returns a numeric exit status or a signal description string. -If you quit, the process is killed with SIGINT, or SIGKILL if you quit again. - -usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &rest ARGS) */) - (ptrdiff_t nargs, Lisp_Object *args) +static Lisp_Object +create_temp_file (ptrdiff_t nargs, Lisp_Object *args) { struct gcpro gcpro1; Lisp_Object filename_string; - register Lisp_Object start, end; - ptrdiff_t count = SPECPDL_INDEX (); - /* Qt denotes we have not yet called Ffind_operation_coding_system. */ - Lisp_Object coding_systems; - Lisp_Object val, *args2; - ptrdiff_t i; + Lisp_Object val, start, end; Lisp_Object tmpdir; if (STRINGP (Vtemporary_file_directory)) @@ -1016,9 +991,7 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r } { - USE_SAFE_ALLOCA; Lisp_Object pattern = Fexpand_file_name (Vtemp_file_name_pattern, tmpdir); - Lisp_Object encoded_tem; char *tempfile; #ifdef WINDOWSNT @@ -1036,39 +1009,30 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r } #endif - encoded_tem = ENCODE_FILE (pattern); - tempfile = SAFE_ALLOCA (SBYTES (encoded_tem) + 1); - memcpy (tempfile, SDATA (encoded_tem), SBYTES (encoded_tem) + 1); - coding_systems = Qt; + filename_string = Fcopy_sequence (ENCODE_FILE (pattern)); + GCPRO1 (filename_string); + tempfile = SSDATA (filename_string); -#if defined HAVE_MKOSTEMP || defined HAVE_MKSTEMP { - int fd, open_errno; + int fd; - block_input (); -# ifdef HAVE_MKOSTEMP +#ifdef HAVE_MKOSTEMP fd = mkostemp (tempfile, O_CLOEXEC); -# else +#elif defined HAVE_MKSTEMP fd = mkstemp (tempfile); -# endif - open_errno = errno; - unblock_input (); +#else + errno = EEXIST; + mktemp (tempfile); + /* INT_MAX denotes success, because close (INT_MAX) does nothing. */ + fd = *tempfile ? INT_MAX : -1; +#endif if (fd < 0) - report_file_errno ("Failed to open temporary file", - Fcons (build_string (tempfile), Qnil), open_errno); + report_file_error ("Failed to open temporary file using pattern", + Fcons (pattern, Qnil)); emacs_close (fd); } -#else - errno = EEXIST; - mktemp (tempfile); - if (!*tempfile) - report_file_error ("Failed to open temporary file using pattern", - Fcons (pattern, Qnil)); -#endif - filename_string = build_string (tempfile); - GCPRO1 (filename_string); - SAFE_FREE (); + record_unwind_protect (delete_temp_file, filename_string); } start = args[0]; @@ -1080,10 +1044,12 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r val = Qraw_text; else { + Lisp_Object coding_systems; + Lisp_Object *args2; USE_SAFE_ALLOCA; SAFE_NALLOCA (args2, 1, nargs + 1); args2[0] = Qcall_process_region; - for (i = 0; i < nargs; i++) args2[i + 1] = args[i]; + memcpy (args2 + 1, args, nargs * sizeof *args); coding_systems = Ffind_operation_coding_system (nargs + 1, args2); val = CONSP (coding_systems) ? XCDR (coding_systems) : Qnil; SAFE_FREE (); @@ -1105,7 +1071,59 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r /* Note that Fcall_process takes care of binding coding-system-for-read. */ - record_unwind_protect (delete_temp_file, filename_string); + RETURN_UNGCPRO (filename_string); +} + +DEFUN ("call-process-region", Fcall_process_region, Scall_process_region, + 3, MANY, 0, + doc: /* Send text from START to END to a synchronous process running PROGRAM. +The remaining arguments are optional. +Delete the text if fourth arg DELETE is non-nil. + +Insert output in BUFFER before point; t means current buffer; nil for + BUFFER means discard it; 0 means discard and don't wait; and `(:file + FILE)', where FILE is a file name string, means that it should be + written to that file (if the file already exists it is overwritten). +BUFFER can also have the form (REAL-BUFFER STDERR-FILE); in that case, +REAL-BUFFER says what to do with standard output, as above, +while STDERR-FILE says what to do with standard error in the child. +STDERR-FILE may be nil (discard standard error output), +t (mix it with ordinary output), or a file name string. + +Sixth arg DISPLAY non-nil means redisplay buffer as output is inserted. +Remaining args are passed to PROGRAM at startup as command args. + +If BUFFER is 0, `call-process-region' returns immediately with value nil. +Otherwise it waits for PROGRAM to terminate +and returns a numeric exit status or a signal description string. +If you quit, the process is killed with SIGINT, or SIGKILL if you quit again. + +usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &rest ARGS) */) + (ptrdiff_t nargs, Lisp_Object *args) +{ + struct gcpro gcpro1; + Lisp_Object filename_string; + ptrdiff_t count = SPECPDL_INDEX (); + Lisp_Object start = args[0]; + Lisp_Object end = args[1]; + bool empty_input; + + if (STRINGP (start)) + empty_input = SCHARS (start) == 0; + else if (NILP (start)) + empty_input = BEG == Z; + else + { + validate_region (&args[0], &args[1]); + start = args[0]; + end = args[1]; + empty_input = XINT (start) == XINT (end); + } + + filename_string = (empty_input + ? build_string (NULL_DEVICE) + : create_temp_file (nargs, args)); + GCPRO1 (filename_string); if (nargs > 3 && !NILP (args[3])) Fdelete_region (start, end); -- 2.39.2