diff options
author | Paul Eggert <eggert@cs.ucla.edu> | 2017-08-12 14:00:17 -0700 |
---|---|---|
committer | Paul Eggert <eggert@cs.ucla.edu> | 2017-08-12 15:14:45 -0700 |
commit | a6ad98ad66e1d0c0dac5f25ba91e11d0cf9da725 (patch) | |
tree | 183748df6a5b71f1b48deecc824eef27939ff2f8 | |
parent | 9eb30cb03613ae158c870d603a05a6a6393dc485 (diff) | |
download | emacs-a6ad98ad66e1d0c0dac5f25ba91e11d0cf9da725.tar.gz |
Improve make-temp-file performance on local files
For the motivation behind this patch, please see Bug#28023 and:
http://emacshorrors.com/posts/make-temp-name.html
Although, given the recent changes to Tramp, the related security
problem in make-temp-file is already fixed, make-temp-file still has
several unnecessary system calls. In the typical case on GNU/Linux,
this patch replaces 8 syscalls (symlink, open, close, readlinkat, uname,
getpid, unlink, umask) by 2 (open, close).
* admin/merge-gnulib (GNULIB_MODULES): Add tempname, now
that Emacs is using it directly.
* configure.ac (AUTO_DEPEND): Remove AC_SYS_LONG_FILE_NAMES;
no longer needed.
* lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate.
* lisp/files.el (files--make-magic-temp-file): Rename from
make-temp-file.
(make-temp-file): Use make-temp-file-internal for
non-magic file names.
* src/fileio.c: Include tempname.h.
(make_temp_name_tbl, make_temp_name_count)
(make_temp_name_count_initialized_p, make_temp_name): Remove.
(Fmake_temp_file_internal): New function.
(Fmake_temp_name): Use it.
* src/filelock.c (get_boot_time): Use Fmake_temp_file_internal
instead of make_temp_name.
-rw-r--r-- | admin/CPP-DEFINES | 1 | ||||
-rwxr-xr-x | admin/merge-gnulib | 4 | ||||
-rw-r--r-- | configure.ac | 3 | ||||
-rw-r--r-- | lib/gnulib.mk.in | 5 | ||||
-rw-r--r-- | lisp/files.el | 9 | ||||
-rw-r--r-- | m4/gnulib-comp.m4 | 13 | ||||
-rw-r--r-- | src/buffer.c | 1 | ||||
-rw-r--r-- | src/fileio.c | 170 | ||||
-rw-r--r-- | src/filelock.c | 13 | ||||
-rw-r--r-- | src/lisp.h | 1 |
10 files changed, 63 insertions, 157 deletions
diff --git a/admin/CPP-DEFINES b/admin/CPP-DEFINES index cead305aee1..10b558d1ada 100644 --- a/admin/CPP-DEFINES +++ b/admin/CPP-DEFINES @@ -205,7 +205,6 @@ HAVE_LIBXML2 HAVE_LIBXMU HAVE_LOCALTIME_R HAVE_LOCAL_SOCKETS -HAVE_LONG_FILE_NAMES HAVE_LONG_LONG_INT HAVE_LRAND48 HAVE_LSTAT diff --git a/admin/merge-gnulib b/admin/merge-gnulib index a16d7fa53ea..7eca64305de 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -39,8 +39,8 @@ GNULIB_MODULES=' manywarnings memrchr minmax mkostemp mktime nstrftime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio - stpcpy strtoimax symlink sys_stat - sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub + stpcpy strtoimax symlink sys_stat sys_time + tempname time time_r time_rz timegm timer-time timespec-add timespec-sub update-copyright unlocked-io utimens vla warnings ' diff --git a/configure.ac b/configure.ac index 9f80620a807..86d5b3e94f9 100644 --- a/configure.ac +++ b/configure.ac @@ -1779,9 +1779,6 @@ if test "$GCC" = yes && test "$ac_enable_autodepend" = yes; then fi AC_SUBST(AUTO_DEPEND) -dnl checks for operating system services -AC_SYS_LONG_FILE_NAMES - #### Choose a window system. ## We leave window_system equal to none if diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in index c5df3f42e47..30986b4ed7f 100644 --- a/lib/gnulib.mk.in +++ b/lib/gnulib.mk.in @@ -21,7 +21,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=setenv --avoid=sigprocmask --avoid=stat --avoid=stdarg --avoid=stdbool --avoid=threadlib --avoid=tzset --avoid=unsetenv --avoid=utime --avoid=utime-h --gnu-make --makefile-name=gnulib.mk.in --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-leading-zeros count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 d-type diffseq dtoastr dtotimespec dup2 environ execinfo explicit_bzero faccessat fcntl fcntl-h fdatasync fdopendir filemode filevercmp flexmember fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog ignore-value intprops largefile lstat manywarnings memrchr minmax mkostemp mktime nstrftime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio stpcpy strtoimax symlink sys_stat sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub unlocked-io update-copyright utimens vla warnings +# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=setenv --avoid=sigprocmask --avoid=stat --avoid=stdarg --avoid=stdbool --avoid=threadlib --avoid=tzset --avoid=unsetenv --avoid=utime --avoid=utime-h --gnu-make --makefile-name=gnulib.mk.in --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-leading-zeros count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 d-type diffseq dtoastr dtotimespec dup2 environ execinfo explicit_bzero faccessat fcntl fcntl-h fdatasync fdopendir filemode filevercmp flexmember fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog ignore-value intprops largefile lstat manywarnings memrchr minmax mkostemp mktime nstrftime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio stpcpy strtoimax symlink sys_stat sys_time tempname time time_r time_rz timegm timer-time timespec-add timespec-sub unlocked-io update-copyright utimens vla warnings MOSTLYCLEANFILES += core *.stackdump @@ -904,7 +904,6 @@ gl_GNULIB_ENABLED_euidaccess = @gl_GNULIB_ENABLED_euidaccess@ gl_GNULIB_ENABLED_getdtablesize = @gl_GNULIB_ENABLED_getdtablesize@ gl_GNULIB_ENABLED_getgroups = @gl_GNULIB_ENABLED_getgroups@ gl_GNULIB_ENABLED_strtoll = @gl_GNULIB_ENABLED_strtoll@ -gl_GNULIB_ENABLED_tempname = @gl_GNULIB_ENABLED_tempname@ gl_LIBOBJS = @gl_LIBOBJS@ gl_LTLIBOBJS = @gl_LTLIBOBJS@ gltests_LIBOBJS = @gltests_LIBOBJS@ @@ -2701,10 +2700,8 @@ endif ## begin gnulib module tempname ifeq (,$(OMIT_GNULIB_MODULE_tempname)) -ifneq (,$(gl_GNULIB_ENABLED_tempname)) libgnu_a_SOURCES += tempname.c -endif EXTRA_DIST += tempname.h endif diff --git a/lisp/files.el b/lisp/files.el index 0fe7f9c522a..19573cdf7b2 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -1407,6 +1407,15 @@ You can then use `write-region' to write new data into the file. If DIR-FLAG is non-nil, create a new empty directory instead of a file. If SUFFIX is non-nil, add that at the end of the file name." + (let ((absolute-prefix (expand-file-name prefix temporary-file-directory))) + (if (find-file-name-handler absolute-prefix 'write-region) + (files--make-magic-temp-file prefix dir-flag suffix) + (make-temp-file-internal absolute-prefix + (if dir-flag t) (or suffix ""))))) + +(defun files--make-magic-temp-file (prefix &optional dir-flag suffix) + "Implement (make-temp-file PREFIX DIR-FLAG SUFFIX). +This implementation works on magic file names." ;; Create temp files with strict access rights. It's easy to ;; loosen them later, whereas it's impossible to close the ;; time-window of loose permissions otherwise. diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index 69d77229bf7..d1089860e19 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -387,6 +387,7 @@ AC_DEFUN([gl_INIT], AC_PROG_MKDIR_P gl_SYS_TYPES_H AC_PROG_MKDIR_P + gl_FUNC_GEN_TEMPNAME gl_HEADER_TIME_H gl_TIME_R if test $HAVE_LOCALTIME_R = 0 || test $REPLACE_LOCALTIME_R = 1; then @@ -424,7 +425,6 @@ AC_DEFUN([gl_INIT], gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7=false gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false gl_gnulib_enabled_strtoll=false - gl_gnulib_enabled_tempname=false gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b () { @@ -560,13 +560,6 @@ AC_DEFUN([gl_INIT], gl_gnulib_enabled_strtoll=true fi } - func_gl_gnulib_m4code_tempname () - { - if ! $gl_gnulib_enabled_tempname; then - gl_FUNC_GEN_TEMPNAME - gl_gnulib_enabled_tempname=true - fi - } func_gl_gnulib_m4code_682e609604ccaac6be382e4ee3a4eaec () { if ! $gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec; then @@ -612,9 +605,6 @@ AC_DEFUN([gl_INIT], if test $REPLACE_LSTAT = 1; then func_gl_gnulib_m4code_dosname fi - if test $HAVE_MKOSTEMP = 0; then - func_gl_gnulib_m4code_tempname - fi if test $HAVE_READLINKAT = 0; then func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b fi @@ -644,7 +634,6 @@ AC_DEFUN([gl_INIT], AM_CONDITIONAL([gl_GNULIB_ENABLED_03e0aaad4cb89ca757653bd367a6ccb7], [$gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7]) AM_CONDITIONAL([gl_GNULIB_ENABLED_6099e9737f757db36c47fa9d9f02e88c], [$gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c]) AM_CONDITIONAL([gl_GNULIB_ENABLED_strtoll], [$gl_gnulib_enabled_strtoll]) - AM_CONDITIONAL([gl_GNULIB_ENABLED_tempname], [$gl_gnulib_enabled_tempname]) AM_CONDITIONAL([gl_GNULIB_ENABLED_682e609604ccaac6be382e4ee3a4eaec], [$gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec]) # End of code from modules m4_ifval(gl_LIBSOURCES_LIST, [ diff --git a/src/buffer.c b/src/buffer.c index 0d0f43e937b..2d508f35cf6 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -1085,7 +1085,6 @@ is first appended to NAME, to speed up finding a non-existent buffer. */) genbase = name; else { - /* Note fileio.c:make_temp_name does random differently. */ char number[sizeof "-999999"]; int i = XFASTINT (Frandom (make_number (999999))); AUTO_STRING_WITH_LEN (lnumber, number, sprintf (number, "-%d", i)); diff --git a/src/fileio.c b/src/fileio.c index 31fd84512e1..b7e3b71a475 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -97,6 +97,7 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ #include <allocator.h> #include <careadlinkat.h> #include <stat-time.h> +#include <tempname.h> #include <binary-io.h> @@ -655,149 +656,67 @@ In Unix-syntax, this function just removes the final slash. */) return val; } -static const char make_temp_name_tbl[64] = -{ - 'A','B','C','D','E','F','G','H', - 'I','J','K','L','M','N','O','P', - 'Q','R','S','T','U','V','W','X', - 'Y','Z','a','b','c','d','e','f', - 'g','h','i','j','k','l','m','n', - 'o','p','q','r','s','t','u','v', - 'w','x','y','z','0','1','2','3', - '4','5','6','7','8','9','-','_' -}; - -static unsigned make_temp_name_count, make_temp_name_count_initialized_p; - -/* Value is a temporary file name starting with PREFIX, a string. +DEFUN ("make-temp-file-internal", Fmake_temp_file_internal, + Smake_temp_file_internal, 3, 3, 0, + doc: /* Generate a new file whose name starts with PREFIX, a string. +Return the name of the generated file. If DIR-FLAG is zero, do not +create the file, just its name. Otherwise, if DIR-FLAG is non-nil, +create an empty directory. The file name should end in SUFFIX. - The Emacs process number forms part of the result, so there is - no danger of generating a name being used by another process. - In addition, this function makes an attempt to choose a name - which has no existing file. To make this work, PREFIX should be - an absolute file name. +Signal an error if the file could not be created. - BASE64_P means add the pid as 3 characters in base64 - encoding. In this case, 6 characters will be added to PREFIX to - form the file name. Otherwise, if Emacs is running on a system - with long file names, add the pid as a decimal number. - - This function signals an error if no unique file name could be - generated. */ - -Lisp_Object -make_temp_name (Lisp_Object prefix, bool base64_p) +This function does not grok magic file names. */) + (Lisp_Object prefix, Lisp_Object dir_flag, Lisp_Object suffix) { - Lisp_Object val, encoded_prefix; - ptrdiff_t len; - printmax_t pid; - char *p, *data; - char pidbuf[INT_BUFSIZE_BOUND (printmax_t)]; - int pidlen; - - CHECK_STRING (prefix); - - /* VAL is created by adding 6 characters to PREFIX. The first - three are the PID of this process, in base 64, and the second - three are incremented if the file already exists. This ensures - 262144 unique file names per PID per PREFIX. */ - - pid = getpid (); - - if (base64_p) - { - pidbuf[0] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[1] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[2] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidlen = 3; - } - else + bool make_temp_name = EQ (dir_flag, make_number (0)); + CHECK_STRING (suffix); + if (!make_temp_name) + prefix = Fexpand_file_name (prefix, Vtemporary_file_directory); + + Lisp_Object encoded_prefix = ENCODE_FILE (prefix); + Lisp_Object encoded_suffix = ENCODE_FILE (suffix); + ptrdiff_t prefix_len = SBYTES (encoded_prefix); + ptrdiff_t suffix_len = SBYTES (encoded_suffix); + if (INT_MAX < suffix_len) + args_out_of_range (prefix, suffix); + int nX = 6; + Lisp_Object val = make_uninit_string (prefix_len + nX + suffix_len); + char *data = SSDATA (val); + memcpy (data, SSDATA (encoded_prefix), prefix_len); + memset (data + prefix_len, 'X', nX); + memcpy (data + prefix_len + nX, SSDATA (encoded_suffix), suffix_len); + int kind = (NILP (dir_flag) ? GT_FILE + : make_temp_name ? GT_NOCREATE + : GT_DIR); + int fd = gen_tempname (data, suffix_len, O_BINARY | O_CLOEXEC, kind); + if (fd < 0 || (NILP (dir_flag) && emacs_close (fd) != 0)) { -#ifdef HAVE_LONG_FILE_NAMES - pidlen = sprintf (pidbuf, "%"pMd, pid); -#else - pidbuf[0] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[1] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[2] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidlen = 3; -#endif - } - - encoded_prefix = ENCODE_FILE (prefix); - len = SBYTES (encoded_prefix); - val = make_uninit_string (len + 3 + pidlen); - data = SSDATA (val); - memcpy (data, SSDATA (encoded_prefix), len); - p = data + len; - - memcpy (p, pidbuf, pidlen); - p += pidlen; - - /* Here we try to minimize useless stat'ing when this function is - invoked many times successively with the same PREFIX. We achieve - this by initializing count to a random value, and incrementing it - afterwards. - - We don't want make-temp-name to be called while dumping, - because then make_temp_name_count_initialized_p would get set - and then make_temp_name_count would not be set when Emacs starts. */ - - if (!make_temp_name_count_initialized_p) - { - make_temp_name_count = time (NULL); - make_temp_name_count_initialized_p = 1; - } - - while (1) - { - unsigned num = make_temp_name_count; - - p[0] = make_temp_name_tbl[num & 63], num >>= 6; - p[1] = make_temp_name_tbl[num & 63], num >>= 6; - p[2] = make_temp_name_tbl[num & 63], num >>= 6; - - /* Poor man's congruential RN generator. Replace with - ++make_temp_name_count for debugging. */ - make_temp_name_count += 25229; - make_temp_name_count %= 225307; - - if (!check_existing (data)) + static char const kind_message[][32] = { - /* We want to return only if errno is ENOENT. */ - if (errno == ENOENT) - return DECODE_FILE (val); - else - /* The error here is dubious, but there is little else we - can do. The alternatives are to return nil, which is - as bad as (and in many cases worse than) throwing the - error, or to ignore the error, which will likely result - in looping through 225307 stat's, which is not only - dog-slow, but also useless since eventually nil would - have to be returned anyway. */ - report_file_error ("Cannot create temporary name for prefix", - prefix); - /* not reached */ - } + [GT_FILE] = "Creating file with prefix", + [GT_DIR] = "Creating directory with prefix", + [GT_NOCREATE] = "Creating file name with prefix" + }; + report_file_error (kind_message[kind], prefix); } + return DECODE_FILE (val); } DEFUN ("make-temp-name", Fmake_temp_name, Smake_temp_name, 1, 1, 0, doc: /* Generate temporary file name (string) starting with PREFIX (a string). -The Emacs process number forms part of the result, so there is no -danger of generating a name being used by another Emacs process -\(so long as only a single host can access the containing directory...). This function tries to choose a name that has no existing file. For this to work, PREFIX should be an absolute file name, and PREFIX and the returned string should both be non-magic. -There is a race condition between calling `make-temp-name' and creating the -file, which opens all kinds of security holes. For that reason, you should -normally use `make-temp-file' instead. */) +There is a race condition between calling `make-temp-name' and +later creating the file, which opens all kinds of security holes. +For that reason, you should normally use `make-temp-file' instead. */) (Lisp_Object prefix) { - return make_temp_name (prefix, 0); + return Fmake_temp_file_internal (prefix, make_number (0), + empty_unibyte_string); } DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0, @@ -6168,6 +6087,7 @@ This includes interactive calls to `delete-file' and defsubr (&Sfile_name_as_directory); defsubr (&Sdirectory_name_p); defsubr (&Sdirectory_file_name); + defsubr (&Smake_temp_file_internal); defsubr (&Smake_temp_name); defsubr (&Sexpand_file_name); defsubr (&Ssubstitute_in_file_name); diff --git a/src/filelock.c b/src/filelock.c index dd8cb28c425..3d6941695ae 100644 --- a/src/filelock.c +++ b/src/filelock.c @@ -206,14 +206,11 @@ get_boot_time (void) WTMP_FILE, counter); if (! NILP (Ffile_exists_p (tempname))) { - /* The utmp functions on mescaline.gnu.org accept only - file names up to 8 characters long. Choose a 2 - character long prefix, and call make_temp_file with - second arg non-zero, so that it will add not more - than 6 characters to the prefix. */ - filename = Fexpand_file_name (build_string ("wt"), - Vtemporary_file_directory); - filename = make_temp_name (filename, 1); + /* The utmp functions on older systems accept only file + names up to 8 bytes long. Choose a 2 byte prefix, so + the 6-byte suffix does not make the name too long. */ + filename = Fmake_temp_file_internal (build_string ("wt"), Qnil, + empty_unibyte_string); CALLN (Fcall_process, build_string ("gzip"), Qnil, list2 (QCfile, filename), Qnil, build_string ("-cd"), tempname); diff --git a/src/lisp.h b/src/lisp.h index 25be5c0ceea..48cf3b30709 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4014,7 +4014,6 @@ extern bool file_directory_p (const char *); extern bool file_accessible_directory_p (Lisp_Object); extern void init_fileio (void); extern void syms_of_fileio (void); -extern Lisp_Object make_temp_name (Lisp_Object, bool); /* Defined in search.c. */ extern void shrink_regexp_cache (void); |