unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#28023: fix make-temp-file race on local host
@ 2017-08-09  5:38 Paul Eggert
  2017-08-09  6:51 ` Michael Albinus
  2017-08-09 16:05 ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Paul Eggert @ 2017-08-09  5:38 UTC (permalink / raw)
  To: 28023

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

Tags: patch
Severity: important

I plan to install the attached patch in master soon, and am sending it for 
review to bug-gnu-emacs first in case there's some problem with it on 
MS-Windows. Although this patch does not fix all instances of the race in Gnu 
Emacs (notably, Tramp access is still affected) it's a good start.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-make-temp-file-race-on-local-files.patch --]
[-- Type: text/x-patch; name="0001-Fix-make-temp-file-race-on-local-files.patch", Size: 22211 bytes --]

From 0f536834ae618adc38883fc331f75a2847a8411c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 8 Aug 2017 22:22:48 -0700
Subject: [PATCH] Fix make-temp-file race on local files

For the motivation behind this patch, please see:
http://emacshorrors.com/posts/make-temp-name.html
The race still exists for magic file names;
fixing this will require Tramp surgery.
* 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.  Add comment describing race bug.
(make-temp-file): Use fileio--make-temp-file for
non-magic files 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.
(Ffileio__make_temp_file): New function.
(Fmake_temp_name): Use it.
* src/filelock.c (get_boot_time): Use Ffileio__make_temp_file
instead of make_temp_name; this avoids a race.
---
 admin/CPP-DEFINES  |   1 -
 admin/merge-gnulib |   4 +-
 configure.ac       |   3 -
 lib/gnulib.mk.in   |   8 +--
 lisp/files.el      |  15 +++++
 m4/gnulib-comp.m4  |  34 +++--------
 src/buffer.c       |   1 -
 src/fileio.c       | 170 ++++++++++++++---------------------------------------
 src/filelock.c     |  11 ++--
 src/lisp.h         |   1 -
 10 files changed, 75 insertions(+), 173 deletions(-)

diff --git a/admin/CPP-DEFINES b/admin/CPP-DEFINES
index cead305aee..10b558d1ad 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 c23e8a40ea..c7a7a1d7c9 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 9f80620a80..86d5b3e94f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1779,9 +1779,6 @@ AC_DEFUN
 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 a385c8c838..8b34ee6354 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
@@ -903,9 +903,7 @@ gl_GNULIB_ENABLED_dosname = @gl_GNULIB_ENABLED_dosname@
 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_secure_getenv = @gl_GNULIB_ENABLED_secure_getenv@
 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@
@@ -1915,9 +1913,7 @@ endif
 ## begin gnulib module secure_getenv
 ifeq (,$(OMIT_GNULIB_MODULE_secure_getenv))
 
-ifneq (,$(gl_GNULIB_ENABLED_secure_getenv))
 
-endif
 EXTRA_DIST += secure_getenv.c
 
 EXTRA_libgnu_a_SOURCES += secure_getenv.c
@@ -2715,10 +2711,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 f2758ab18c..3ed36d24db 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1417,6 +1417,19 @@ make-temp-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)))
+    ;; FIXME: Establish make-temp-file file name handlers,
+    ;; to create temporary files and directories without races.
+    ;; For now, if the prefix is magic, fall back on the traditional,
+    ;; insecure way to create temporary files.
+    (if (find-file-name-handler absolute-prefix 'write-region)
+        (files--make-magic-temp-file prefix dir-flag suffix)
+      (fileio--make-temp-file 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),
+even if the result is a magic file name."
   ;; 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.
@@ -1435,6 +1448,8 @@ make-temp-file
 		       (setq file (concat file suffix)))
 		   (if dir-flag
 		       (make-directory file)
+                     ;; FIXME: The 'excl is only aspirational; it is
+                     ;; actually a no-op if FILE is a magic file name.
 		     (write-region "" nil file nil 'silent nil 'excl))
 		   nil)
 	       (file-already-exists t))
diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4
index 188c116c85..fc86237ee9 100644
--- a/m4/gnulib-comp.m4
+++ b/m4/gnulib-comp.m4
@@ -347,6 +347,12 @@ AC_DEFUN
     AC_LIBOBJ([readlinkat])
   fi
   gl_UNISTD_MODULE_INDICATOR([readlinkat])
+  gl_FUNC_SECURE_GETENV
+  if test $HAVE_SECURE_GETENV = 0; then
+    AC_LIBOBJ([secure_getenv])
+    gl_PREREQ_SECURE_GETENV
+  fi
+  gl_STDLIB_MODULE_INDICATOR([secure_getenv])
   gl_FUNC_SIG2STR
   if test $ac_cv_func_sig2str = no; then
     AC_LIBOBJ([sig2str])
@@ -388,6 +394,7 @@ AC_DEFUN
   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,9 +431,7 @@ AC_DEFUN
   gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31=false
   gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7=false
   gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false
-  gl_gnulib_enabled_secure_getenv=false
   gl_gnulib_enabled_strtoll=false
-  gl_gnulib_enabled_tempname=false
   gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false
   func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b ()
   {
@@ -550,18 +555,6 @@ AC_DEFUN
       gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=true
     fi
   }
-  func_gl_gnulib_m4code_secure_getenv ()
-  {
-    if ! $gl_gnulib_enabled_secure_getenv; then
-      gl_FUNC_SECURE_GETENV
-      if test $HAVE_SECURE_GETENV = 0; then
-        AC_LIBOBJ([secure_getenv])
-        gl_PREREQ_SECURE_GETENV
-      fi
-      gl_STDLIB_MODULE_INDICATOR([secure_getenv])
-      gl_gnulib_enabled_secure_getenv=true
-    fi
-  }
   func_gl_gnulib_m4code_strtoll ()
   {
     if ! $gl_gnulib_enabled_strtoll; then
@@ -574,14 +567,6 @@ AC_DEFUN
       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
-      func_gl_gnulib_m4code_secure_getenv
-    fi
-  }
   func_gl_gnulib_m4code_682e609604ccaac6be382e4ee3a4eaec ()
   {
     if ! $gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec; then
@@ -627,9 +612,6 @@ AC_DEFUN
   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
@@ -658,9 +640,7 @@ AC_DEFUN
   AM_CONDITIONAL([gl_GNULIB_ENABLED_5264294aa0a5557541b53c8c741f7f31], [$gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31])
   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_secure_getenv], [$gl_gnulib_enabled_secure_getenv])
   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 0d0f43e937..2d508f35cf 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 15845e3914..1ef9bfa82b 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>
 
@@ -623,149 +624,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 ("fileio--make-temp-file", Ffileio__make_temp_file,
+       Sfileio__make_temp_file, 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 Ffileio__make_temp_file (prefix, make_number (0),
+				  empty_unibyte_string);
 }
 
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
@@ -6167,6 +6086,7 @@ This includes interactive calls to `delete-file' and
   defsubr (&Sunhandled_file_name_directory);
   defsubr (&Sfile_name_as_directory);
   defsubr (&Sdirectory_file_name);
+  defsubr (&Sfileio__make_temp_file);
   defsubr (&Smake_temp_name);
   defsubr (&Sexpand_file_name);
   defsubr (&Ssubstitute_in_file_name);
diff --git a/src/filelock.c b/src/filelock.c
index dd8cb28c42..de0ca01052 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -206,14 +206,13 @@ 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.  */
+	      /* 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 = Fexpand_file_name (build_string ("wt"),
 					    Vtemporary_file_directory);
-	      filename = make_temp_name (filename, 1);
+	      filename = Ffileio__make_temp_file (filename, 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 4de6fc85ec..a65cc9684a 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4013,7 +4013,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);
-- 
2.13.3


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09  5:38 bug#28023: fix make-temp-file race on local host Paul Eggert
@ 2017-08-09  6:51 ` Michael Albinus
  2017-08-09  9:22   ` Paul Eggert
  2017-08-09 16:05 ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Albinus @ 2017-08-09  6:51 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

Paul Eggert <eggert@cs.ucla.edu> writes:

Hi Paul,

> I plan to install the attached patch in master soon, and am sending it
> for review to bug-gnu-emacs first in case there's some problem with it
> on MS-Windows. Although this patch does not fix all instances of the
> race in Gnu Emacs (notably, Tramp access is still affected) it's a
> good start.

I haven't followed the whole thread, sorry. Could you pls elaborate,
what you do expect from Tramp?

Best regards, Michael.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09  6:51 ` Michael Albinus
@ 2017-08-09  9:22   ` Paul Eggert
  2017-08-09 11:21     ` Michael Albinus
  2017-08-12 10:35     ` Michael Albinus
  0 siblings, 2 replies; 23+ messages in thread
From: Paul Eggert @ 2017-08-09  9:22 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 28023

Michael Albinus wrote:
> what you do expect from Tramp?

Tramp needs to support a new method make-temp-file that creates a file (or 
directory) atomically, as make-temp-file does locally now. Tramp also needs to 
support the excl flag of write-region (currently it ignores that flag). I was 
planning to write this up as a bug report after the patch goes in.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09  9:22   ` Paul Eggert
@ 2017-08-09 11:21     ` Michael Albinus
  2017-08-13  7:21       ` Paul Eggert
  2017-08-12 10:35     ` Michael Albinus
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Albinus @ 2017-08-09 11:21 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

Paul Eggert <eggert@cs.ucla.edu> writes:

> Tramp needs to support a new method make-temp-file that creates a file
> (or directory) atomically, as make-temp-file does locally now.

That means, make-temp-file shall be converted into a magic file name
operation. I'll do my best, but I don't know whether we could get an
implementation for all Tramp methods w/o a race condition.

> Tramp also needs to support the excl flag of write-region (currently
> it ignores that flag).

This sounds trivial. And yes, it will help.

> I was planning to write this up as a bug report after the patch goes
> in.

Pls do.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09  5:38 bug#28023: fix make-temp-file race on local host Paul Eggert
  2017-08-09  6:51 ` Michael Albinus
@ 2017-08-09 16:05 ` Eli Zaretskii
  2017-08-09 18:47   ` Paul Eggert
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-09 16:05 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 8 Aug 2017 22:38:05 -0700
> 
> I plan to install the attached patch in master soon, and am sending it for 
> review to bug-gnu-emacs first in case there's some problem with it on 
> MS-Windows.

Thanks for the heads-up.  Unfortunately, it isn't as simple as we'd
like it to be, because Gnulib doesn't support file names encoded in
UTF-8 (or any other encoding except the current system codepage) on
MS-Windows, while Emacs does.

We could resolve this problem by providing a way for the Windows build
to specify its own replacements for 'open', 'mkdir', and 'lstat'
(Emacs already have such replacements in w32.c), which tempname.c
calls, but that would require minor changes in Gnulib's tempname.c.
Or we could steal the relevant code from tempname.c into a
Windows-specific implementation in w32.c, and refrain from using the
Gnulib version on Windows.  Which way would you prefer to go?  Or
maybe you have yet another idea?

> @@ -1915,9 +1913,7 @@ endif
>  ## begin gnulib module secure_getenv
>  ifeq (,$(OMIT_GNULIB_MODULE_secure_getenv))
>  
> -ifneq (,$(gl_GNULIB_ENABLED_secure_getenv))
>  
> -endif
>  EXTRA_DIST += secure_getenv.c

This seems to say that we will sometimes use secure_getenv, but the
relevant code in tempname.c is conditioned on _LIBC, which AFAIU is
only true when building glibc, so why do we need that?  Or did I miss
something?





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09 16:05 ` Eli Zaretskii
@ 2017-08-09 18:47   ` Paul Eggert
  2017-08-09 19:09     ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-08-09 18:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28023

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

On 08/09/2017 09:05 AM, Eli Zaretskii wrote:
> We could resolve this problem by providing a way for the Windows build
> to specify its own replacements for 'open', 'mkdir', and 'lstat'
> (Emacs already have such replacements in w32.c), which tempname.c
> calls, but that would require minor changes in Gnulib's tempname.c.

This sounds like a better way to go. What minor changes would be needed? 
tempname.c already includes config.h, which includes ms-w32.h, which 
redefines open, mkdir, and lstat, so I thought it would work as-is.

> This seems to say that we will sometimes use secure_getenv, but the
> relevant code in tempname.c is conditioned on _LIBC, which AFAIU is
> only true when building glibc, so why do we need that?  Or did I miss
> something?
No, good catch, Emacs never needs secure_getenv. I fixed Gnulib, 
installed the attached patch to propagate that fix, and will adjust the 
proposed make-temp-file patch accordingly.


[-- Attachment #2: 0001-Merge-from-gnulib.patch --]
[-- Type: text/x-patch, Size: 14696 bytes --]

From 8daab9f4a0ff6dbcfbd0c6f9262e41753de3484b Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 9 Aug 2017 11:38:04 -0700
Subject: [PATCH] Merge from gnulib

This incorporates:
2017-08-09 tempname: do not depend on secure_getenv
2017-08-08 extensions: add _OPENBSD_SOURCE
2017-08-06 manywarnings: Add support for C++
2017-08-06 warnings, manywarnings: Add support for multiple languages
* admin/merge-gnulib: Don't use m4/manywarnings-c++.m4.
* lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate.
* lib/secure_getenv.c, m4/secure_getenv.m4: Remove.
* lib/tempname.c, m4/extensions.m4, m4/manywarnings.m4, m4/warnings.m4:
Copy from gnulib.
---
 admin/merge-gnulib  |  1 +
 lib/gnulib.mk.in    | 14 --------------
 lib/secure_getenv.c | 54 -----------------------------------------------------
 lib/tempname.c      |  1 -
 m4/extensions.m4    |  7 ++++++-
 m4/gnulib-comp.m4   | 19 +------------------
 m4/manywarnings.m4  | 18 +++++++++++++++++-
 m4/secure_getenv.m4 | 26 --------------------------
 m4/warnings.m4      | 47 +++++++++++++++++++++++++++++++++++-----------
 9 files changed, 61 insertions(+), 126 deletions(-)
 delete mode 100644 lib/secure_getenv.c
 delete mode 100644 m4/secure_getenv.m4

diff --git a/admin/merge-gnulib b/admin/merge-gnulib
index c23e8a40ea..a16d7fa53e 100755
--- a/admin/merge-gnulib
+++ b/admin/merge-gnulib
@@ -106,6 +106,7 @@ avoided_flags=
 rm -- "$src"lib/gl_openssl.h "$src"m4/fcntl-o.m4 \
       "$src"m4/gl-openssl.m4 \
       "$src"m4/gnulib-cache.m4 "$src"m4/gnulib-tool.m4 \
+      "$src"m4/manywarnings-c++.m4 \
       "$src"m4/warn-on-use.m4 "$src"m4/wint_t.m4 &&
 cp -- "$gnulib_srcdir"/build-aux/texinfo.tex "$src"doc/misc &&
 cp -- "$gnulib_srcdir"/build-aux/config.guess \
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index a385c8c838..c5df3f42e4 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -903,7 +903,6 @@ gl_GNULIB_ENABLED_dosname = @gl_GNULIB_ENABLED_dosname@
 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_secure_getenv = @gl_GNULIB_ENABLED_secure_getenv@
 gl_GNULIB_ENABLED_strtoll = @gl_GNULIB_ENABLED_strtoll@
 gl_GNULIB_ENABLED_tempname = @gl_GNULIB_ENABLED_tempname@
 gl_LIBOBJS = @gl_LIBOBJS@
@@ -1912,19 +1911,6 @@ EXTRA_DIST += root-uid.h
 endif
 ## end   gnulib module root-uid
 
-## begin gnulib module secure_getenv
-ifeq (,$(OMIT_GNULIB_MODULE_secure_getenv))
-
-ifneq (,$(gl_GNULIB_ENABLED_secure_getenv))
-
-endif
-EXTRA_DIST += secure_getenv.c
-
-EXTRA_libgnu_a_SOURCES += secure_getenv.c
-
-endif
-## end   gnulib module secure_getenv
-
 ## begin gnulib module sig2str
 ifeq (,$(OMIT_GNULIB_MODULE_sig2str))
 
diff --git a/lib/secure_getenv.c b/lib/secure_getenv.c
deleted file mode 100644
index df53dea0b2..0000000000
--- a/lib/secure_getenv.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/* Look up an environment variable, returning NULL in insecure situations.
-
-   Copyright 2013-2017 Free Software Foundation, Inc.
-
-   This program is free software: you can redistribute it and/or modify it
-   under the terms of the GNU General Public License as published
-   by the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
-
-#include <config.h>
-
-#include <stdlib.h>
-
-#if !HAVE___SECURE_GETENV
-# if HAVE_ISSETUGID || (HAVE_GETUID && HAVE_GETEUID && HAVE_GETGID && HAVE_GETEGID)
-#  include <unistd.h>
-# endif
-#endif
-
-char *
-secure_getenv (char const *name)
-{
-#if HAVE___SECURE_GETENV /* glibc */
-  return __secure_getenv (name);
-#elif HAVE_ISSETUGID /* OS X, FreeBSD, NetBSD, OpenBSD */
-  if (issetugid ())
-    return NULL;
-  return getenv (name);
-#elif HAVE_GETUID && HAVE_GETEUID && HAVE_GETGID && HAVE_GETEGID /* other Unix */
-  if (geteuid () != getuid () || getegid () != getgid ())
-    return NULL;
-  return getenv (name);
-#elif (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ /* native Windows */
-  /* On native Windows, there is no such concept as setuid or setgid binaries.
-     - Programs launched as system services have high privileges, but they don't
-       inherit environment variables from a user.
-     - Programs launched by a user with "Run as Administrator" have high
-       privileges and use the environment variables, but the user has been asked
-       whether he agrees.
-     - Programs launched by a user without "Run as Administrator" cannot gain
-       high privileges, therefore there is no risk. */
-  return getenv (name);
-#else
-  return NULL;
-#endif
-}
diff --git a/lib/tempname.c b/lib/tempname.c
index 9c4a3c2a54..c274b8dd4e 100644
--- a/lib/tempname.c
+++ b/lib/tempname.c
@@ -69,7 +69,6 @@
 # define __mkdir mkdir
 # define __open open
 # define __lxstat64(version, file, buf) lstat (file, buf)
-# define __secure_getenv secure_getenv
 #endif
 
 #ifdef _LIBC
diff --git a/m4/extensions.m4 b/m4/extensions.m4
index c60f537db1..0c16bb832f 100644
--- a/m4/extensions.m4
+++ b/m4/extensions.m4
@@ -1,4 +1,4 @@
-# serial 15  -*- Autoconf -*-
+# serial 16  -*- Autoconf -*-
 # Enable extensions on systems that normally disable them.
 
 # Copyright (C) 2003, 2006-2017 Free Software Foundation, Inc.
@@ -68,6 +68,10 @@ AC_DEFUN_ONCE
 #ifndef _GNU_SOURCE
 # undef _GNU_SOURCE
 #endif
+/* Enable OpenBSD extensions on NetBSD.  */
+#ifndef _OPENBSD_SOURCE
+# undef _OPENBSD_SOURCE
+#endif
 /* Enable threading extensions on Solaris.  */
 #ifndef _POSIX_PTHREAD_SEMANTICS
 # undef _POSIX_PTHREAD_SEMANTICS
@@ -128,6 +132,7 @@ AC_DEFUN_ONCE
   AC_DEFINE([_ALL_SOURCE])
   AC_DEFINE([_DARWIN_C_SOURCE])
   AC_DEFINE([_GNU_SOURCE])
+  AC_DEFINE([_OPENBSD_SOURCE])
   AC_DEFINE([_POSIX_PTHREAD_SEMANTICS])
   AC_DEFINE([__STDC_WANT_IEC_60559_ATTRIBS_EXT__])
   AC_DEFINE([__STDC_WANT_IEC_60559_BFP_EXT__])
diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4
index 188c116c85..69d77229bf 100644
--- a/m4/gnulib-comp.m4
+++ b/m4/gnulib-comp.m4
@@ -124,7 +124,6 @@ AC_DEFUN
   # Code from module readlink:
   # Code from module readlinkat:
   # Code from module root-uid:
-  # Code from module secure_getenv:
   # Code from module sig2str:
   # Code from module signal-h:
   # Code from module snippet/_Noreturn:
@@ -424,7 +423,6 @@ AC_DEFUN
   gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31=false
   gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7=false
   gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false
-  gl_gnulib_enabled_secure_getenv=false
   gl_gnulib_enabled_strtoll=false
   gl_gnulib_enabled_tempname=false
   gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false
@@ -550,18 +548,6 @@ AC_DEFUN
       gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=true
     fi
   }
-  func_gl_gnulib_m4code_secure_getenv ()
-  {
-    if ! $gl_gnulib_enabled_secure_getenv; then
-      gl_FUNC_SECURE_GETENV
-      if test $HAVE_SECURE_GETENV = 0; then
-        AC_LIBOBJ([secure_getenv])
-        gl_PREREQ_SECURE_GETENV
-      fi
-      gl_STDLIB_MODULE_INDICATOR([secure_getenv])
-      gl_gnulib_enabled_secure_getenv=true
-    fi
-  }
   func_gl_gnulib_m4code_strtoll ()
   {
     if ! $gl_gnulib_enabled_strtoll; then
@@ -579,7 +565,6 @@ AC_DEFUN
     if ! $gl_gnulib_enabled_tempname; then
       gl_FUNC_GEN_TEMPNAME
       gl_gnulib_enabled_tempname=true
-      func_gl_gnulib_m4code_secure_getenv
     fi
   }
   func_gl_gnulib_m4code_682e609604ccaac6be382e4ee3a4eaec ()
@@ -658,7 +643,6 @@ AC_DEFUN
   AM_CONDITIONAL([gl_GNULIB_ENABLED_5264294aa0a5557541b53c8c741f7f31], [$gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31])
   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_secure_getenv], [$gl_gnulib_enabled_secure_getenv])
   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])
@@ -907,7 +891,6 @@ AC_DEFUN
   lib/readlink.c
   lib/readlinkat.c
   lib/root-uid.h
-  lib/secure_getenv.c
   lib/set-permissions.c
   lib/sha1.c
   lib/sha1.h
@@ -1008,6 +991,7 @@ AC_DEFUN
   m4/localtime-buffer.m4
   m4/longlong.m4
   m4/lstat.m4
+  m4/manywarnings-c++.m4
   m4/manywarnings.m4
   m4/md5.m4
   m4/memrchr.m4
@@ -1024,7 +1008,6 @@ AC_DEFUN
   m4/putenv.m4
   m4/readlink.m4
   m4/readlinkat.m4
-  m4/secure_getenv.m4
   m4/sha1.m4
   m4/sha256.m4
   m4/sha512.m4
diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4
index 6a8939b2c1..a3d255a940 100644
--- a/m4/manywarnings.m4
+++ b/m4/manywarnings.m4
@@ -1,4 +1,4 @@
-# manywarnings.m4 serial 10
+# manywarnings.m4 serial 11
 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -33,8 +33,16 @@ AC_DEFUN
 # Add all documented GCC warning parameters to variable VARIABLE.
 # Note that you need to test them using gl_WARN_ADD if you want to
 # make sure your gcc understands it.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_MANYWARN_ALL_GCC],
+[_AC_LANG_DISPATCH([$0], _AC_LANG, $@)])
+
+# Specialization for _AC_LANG = C.
+AC_DEFUN([gl_MANYWARN_ALL_GCC(C)],
 [
+  AC_LANG_PUSH([C])
+
   dnl First, check for some issues that only occur when combining multiple
   dnl gcc warning categories.
   AC_REQUIRE([AC_PROG_CC])
@@ -303,4 +311,12 @@ AC_DEFUN
   fi
 
   $1=$gl_manywarn_set
+
+  AC_LANG_POP([C])
+])
+
+# Specialization for _AC_LANG = C++.
+AC_DEFUN([gl_MANYWARN_ALL_GCC(C++)],
+[
+  gl_MANYWARN_ALL_GCC_CXX_IMPL([$1])
 ])
diff --git a/m4/secure_getenv.m4 b/m4/secure_getenv.m4
deleted file mode 100644
index 6bd4afd9c1..0000000000
--- a/m4/secure_getenv.m4
+++ /dev/null
@@ -1,26 +0,0 @@
-# Look up an environment variable more securely.
-dnl Copyright 2013-2017 Free Software Foundation, Inc.
-dnl This file is free software; the Free Software Foundation
-dnl gives unlimited permission to copy and/or distribute it,
-dnl with or without modifications, as long as this notice is preserved.
-
-AC_DEFUN([gl_FUNC_SECURE_GETENV],
-[
-  dnl Persuade glibc <stdlib.h> to declare secure_getenv().
-  AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS])
-
-  AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
-  AC_CHECK_FUNCS_ONCE([secure_getenv])
-  if test $ac_cv_func_secure_getenv = no; then
-    HAVE_SECURE_GETENV=0
-  fi
-])
-
-# Prerequisites of lib/secure_getenv.c.
-AC_DEFUN([gl_PREREQ_SECURE_GETENV], [
-  AC_CHECK_FUNCS([__secure_getenv])
-  if test $ac_cv_func___secure_getenv = no; then
-    AC_CHECK_FUNCS([issetugid])
-  fi
-  AC_CHECK_FUNCS_ONCE([getuid geteuid getgid getegid])
-])
diff --git a/m4/warnings.m4 b/m4/warnings.m4
index e697174edd..aa2735b77f 100644
--- a/m4/warnings.m4
+++ b/m4/warnings.m4
@@ -1,4 +1,4 @@
-# warnings.m4 serial 11
+# warnings.m4 serial 12
 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -20,10 +20,12 @@
 # -----------------------------------------------------------------
 # Check if the compiler supports OPTION when compiling PROGRAM.
 #
-# FIXME: gl_Warn must be used unquoted until we can assume Autoconf
-# 2.64 or newer.
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_COMPILER_OPTION_IF],
-[AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
+[
+dnl FIXME: gl_Warn must be used unquoted until we can assume Autoconf
+dnl 2.64 or newer.
+AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl
 AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl
 AS_LITERAL_IF([$1],
   [m4_pushdef([gl_Positive], m4_bpatsubst([$1], [^-Wno-], [-W]))],
@@ -51,27 +53,50 @@ AC_DEFUN
 # ------------------------------
 # Clang doesn't complain about unknown warning options unless one also
 # specifies -Wunknown-warning-option -Werror.  Detect this.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS],
+[_AC_LANG_DISPATCH([$0], _AC_LANG, $@)])
+
+# Specialization for _AC_LANG = C. This macro can be AC_REQUIREd.
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)],
+[
+  AC_LANG_PUSH([C])
+  gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL
+  AC_LANG_POP([C])
+])
+
+# Specialization for _AC_LANG = C++. This macro can be AC_REQUIREd.
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C++)],
+[
+  AC_LANG_PUSH([C++])
+  gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL
+  AC_LANG_POP([C++])
+])
+
+AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL],
 [gl_COMPILER_OPTION_IF([-Werror -Wunknown-warning-option],
    [gl_unknown_warnings_are_errors='-Wunknown-warning-option -Werror'],
    [gl_unknown_warnings_are_errors=])])
 
-# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS],
+# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS/WARN_CXXFLAGS],
 #             [PROGRAM = AC_LANG_PROGRAM()])
-# ---------------------------------------------
-# Adds parameter to WARN_CFLAGS if the compiler supports it when
-# compiling PROGRAM.  For example, gl_WARN_ADD([-Wparentheses]).
+# -----------------------------------------------------------
+# Adds parameter to WARN_CFLAGS/WARN_CXXFLAGS if the compiler supports it
+# when compiling PROGRAM.  For example, gl_WARN_ADD([-Wparentheses]).
 #
 # If VARIABLE is a variable name, AC_SUBST it.
+#
+# The effects of this macro depend on the current language (_AC_LANG).
 AC_DEFUN([gl_WARN_ADD],
-[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS])
+[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS(]_AC_LANG[)])
 gl_COMPILER_OPTION_IF([$1],
-  [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_CFLAGS]], [[$2]]), [" $1"])],
+  [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_]_AC_LANG_PREFIX[FLAGS]], [[$2]]), [" $1"])],
   [],
   [$3])
 m4_ifval([$2],
          [AS_LITERAL_IF([$2], [AC_SUBST([$2])])],
-         [AC_SUBST([WARN_CFLAGS])])dnl
+         [AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])])dnl
 ])
 
 # Local Variables:
-- 
2.13.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09 18:47   ` Paul Eggert
@ 2017-08-09 19:09     ` Eli Zaretskii
  2017-08-09 23:36       ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-09 19:09 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

> Cc: 28023@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 9 Aug 2017 11:47:36 -0700
> 
> On 08/09/2017 09:05 AM, Eli Zaretskii wrote:
> > We could resolve this problem by providing a way for the Windows build
> > to specify its own replacements for 'open', 'mkdir', and 'lstat'
> > (Emacs already have such replacements in w32.c), which tempname.c
> > calls, but that would require minor changes in Gnulib's tempname.c.
> 
> This sounds like a better way to go. What minor changes would be needed? 
> tempname.c already includes config.h, which includes ms-w32.h, which 
> redefines open, mkdir, and lstat, so I thought it would work as-is.

Perhaps you are right, and it will "just work".  I will have to try
(for now, I just looked at the sources and the changes, but didn't try
building).

> > This seems to say that we will sometimes use secure_getenv, but the
> > relevant code in tempname.c is conditioned on _LIBC, which AFAIU is
> > only true when building glibc, so why do we need that?  Or did I miss
> > something?
> No, good catch, Emacs never needs secure_getenv. I fixed Gnulib, 
> installed the attached patch to propagate that fix, and will adjust the 
> proposed make-temp-file patch accordingly.

Thanks.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09 19:09     ` Eli Zaretskii
@ 2017-08-09 23:36       ` Paul Eggert
  2017-08-10 15:35         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-08-09 23:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28023

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

Eli Zaretskii wrote:
> Perhaps you are right, and it will "just work".  I will have to try
> (for now, I just looked at the sources and the changes, but didn't try
> building).

It looks to me like it should just work. You might also consider the second 
attached patch: it removes the MS-Windows implementation of mkostemp, since the 
generic code should just work as well. I've tested only the first attached 
patch, though: it is basically the same as before except it's rebased to current 
master.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-make-temp-file-race-on-local-files.patch --]
[-- Type: text/x-patch; name="0001-Fix-make-temp-file-race-on-local-files.patch", Size: 20531 bytes --]

From 7fe4062aebf56a7de1ae389907098cd461f8d485 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 9 Aug 2017 16:34:40 -0700
Subject: [PATCH 1/2] Fix make-temp-file race on local files

For the motivation behind this patch, please see Bug#28023 and:
http://emacshorrors.com/posts/make-temp-name.html
The race still exists for magic file names;
fixing this will require Tramp surgery.
* 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.  Add comment describing race bug.
(make-temp-file): Use fileio--make-temp-file for
non-magic files 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.
(Ffileio__make_temp_file): New function.
(Fmake_temp_name): Use it.
* src/filelock.c (get_boot_time): Use Ffileio__make_temp_file
instead of make_temp_name; this avoids a race.
---
 admin/CPP-DEFINES  |   1 -
 admin/merge-gnulib |   4 +-
 configure.ac       |   3 -
 lib/gnulib.mk.in   |   5 +-
 lisp/files.el      |  15 +++++
 m4/gnulib-comp.m4  |  13 +---
 src/buffer.c       |   1 -
 src/fileio.c       | 170 ++++++++++++++---------------------------------------
 src/filelock.c     |  13 ++--
 src/lisp.h         |   1 -
 10 files changed, 69 insertions(+), 157 deletions(-)

diff --git a/admin/CPP-DEFINES b/admin/CPP-DEFINES
index cead305aee..10b558d1ad 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 a16d7fa53e..7eca64305d 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 9f80620a80..86d5b3e94f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1779,9 +1779,6 @@ AC_DEFUN
 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 c5df3f42e4..30986b4ed7 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 f2758ab18c..3ed36d24db 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1417,6 +1417,19 @@ make-temp-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)))
+    ;; FIXME: Establish make-temp-file file name handlers,
+    ;; to create temporary files and directories without races.
+    ;; For now, if the prefix is magic, fall back on the traditional,
+    ;; insecure way to create temporary files.
+    (if (find-file-name-handler absolute-prefix 'write-region)
+        (files--make-magic-temp-file prefix dir-flag suffix)
+      (fileio--make-temp-file 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),
+even if the result is a magic file name."
   ;; 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.
@@ -1435,6 +1448,8 @@ make-temp-file
 		       (setq file (concat file suffix)))
 		   (if dir-flag
 		       (make-directory file)
+                     ;; FIXME: The 'excl is only aspirational; it is
+                     ;; actually a no-op if FILE is a magic file name.
 		     (write-region "" nil file nil 'silent nil 'excl))
 		   nil)
 	       (file-already-exists t))
diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4
index 69d77229bf..d1089860e1 100644
--- a/m4/gnulib-comp.m4
+++ b/m4/gnulib-comp.m4
@@ -387,6 +387,7 @@ AC_DEFUN
   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_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_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
   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
   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 0d0f43e937..2d508f35cf 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 15845e3914..1ef9bfa82b 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>
 
@@ -623,149 +624,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 ("fileio--make-temp-file", Ffileio__make_temp_file,
+       Sfileio__make_temp_file, 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 Ffileio__make_temp_file (prefix, make_number (0),
+				  empty_unibyte_string);
 }
 
 DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0,
@@ -6167,6 +6086,7 @@ This includes interactive calls to `delete-file' and
   defsubr (&Sunhandled_file_name_directory);
   defsubr (&Sfile_name_as_directory);
   defsubr (&Sdirectory_file_name);
+  defsubr (&Sfileio__make_temp_file);
   defsubr (&Smake_temp_name);
   defsubr (&Sexpand_file_name);
   defsubr (&Ssubstitute_in_file_name);
diff --git a/src/filelock.c b/src/filelock.c
index dd8cb28c42..5614657985 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 = Ffileio__make_temp_file (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 4de6fc85ec..a65cc9684a 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4013,7 +4013,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);
-- 
2.13.4


[-- Attachment #3: 0002-Remove-ms-w32-mkostemp.patch --]
[-- Type: text/x-patch, Size: 5692 bytes --]

From 130f81bd123de154be5247d3237610d1eeab5dca Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 9 Aug 2017 16:34:40 -0700
Subject: [PATCH 2/2] Remove ms-w32 mkostemp

* lib-src/ntlib.c, nt/inc/ms-w32.h, src/w32.c (mkostemp):
* nt/gnulib-cfg.mk (OMIT_GNULIB_MODULE_mkostemp):
* nt/mingw-cfg.site (ac_cv_func_mkostemp):
Remove, since the generic mkostemp should work on MS-Windows.
---
 lib-src/ntlib.c   | 56 -------------------------------------------------------
 nt/gnulib-cfg.mk  |  1 -
 nt/inc/ms-w32.h   |  2 --
 nt/mingw-cfg.site |  1 -
 src/w32.c         | 55 ------------------------------------------------------
 5 files changed, 115 deletions(-)

diff --git a/lib-src/ntlib.c b/lib-src/ntlib.c
index 78ba9061f6..078a43e79c 100644
--- a/lib-src/ntlib.c
+++ b/lib-src/ntlib.c
@@ -37,7 +37,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 char *sys_ctime (const time_t *);
 FILE *sys_fopen (const char *, const char *);
 int sys_chdir (const char *);
-int mkostemp (char *, int);
 int sys_rename (const char *, const char *);
 
 /* MinGW64 defines _TIMEZONE_DEFINED and defines 'struct timespec' in
@@ -396,61 +395,6 @@ lstat (const char * path, struct stat * buf)
   return stat (path, buf);
 }
 
-/* Implementation of mkostemp for MS-Windows, to avoid race conditions
-   when using mktemp.  Copied from w32.c.
-
-   This is used only in update-game-score.c.  It is overkill for that
-   use case, since update-game-score renames the temporary file into
-   the game score file, which isn't atomic on MS-Windows anyway, when
-   the game score already existed before running the program, which it
-   almost always does.  But using a simpler implementation just to
-   make a point is uneconomical...  */
-
-int
-mkostemp (char * template, int flags)
-{
-  char * p;
-  int i, fd = -1;
-  unsigned uid = GetCurrentThreadId ();
-  int save_errno = errno;
-  static char first_char[] = "abcdefghijklmnopqrstuvwyz0123456789!%-_@#";
-
-  errno = EINVAL;
-  if (template == NULL)
-    return -1;
-
-  p = template + strlen (template);
-  i = 5;
-  /* replace up to the last 5 X's with uid in decimal */
-  while (--p >= template && p[0] == 'X' && --i >= 0)
-    {
-      p[0] = '0' + uid % 10;
-      uid /= 10;
-    }
-
-  if (i < 0 && p[0] == 'X')
-    {
-      i = 0;
-      do
-	{
-	  p[0] = first_char[i];
-	  if ((fd = open (template,
-			  flags | _O_CREAT | _O_EXCL | _O_RDWR,
-			  S_IRUSR | S_IWUSR)) >= 0
-	      || errno != EEXIST)
-	    {
-	      if (fd >= 0)
-		errno = save_errno;
-	      return fd;
-	    }
-	}
-      while (++i < sizeof (first_char));
-    }
-
-  /* Template is badly formed or else we can't generate a unique name.  */
-  return -1;
-}
-
 /* On Windows, you cannot rename into an existing file.  */
 int
 sys_rename (const char *from, const char *to)
diff --git a/nt/gnulib-cfg.mk b/nt/gnulib-cfg.mk
index 175329fb9e..ad798bbe37 100644
--- a/nt/gnulib-cfg.mk
+++ b/nt/gnulib-cfg.mk
@@ -50,7 +50,6 @@ OMIT_GNULIB_MODULE_dirfd =
 OMIT_GNULIB_MODULE_fcntl = true
 OMIT_GNULIB_MODULE_fcntl-h = true
 OMIT_GNULIB_MODULE_inttypes-incomplete = true
-OMIT_GNULIB_MODULE_mkostemp = true
 OMIT_GNULIB_MODULE_pipe2 = true
 OMIT_GNULIB_MODULE_secure_getenv = true
 OMIT_GNULIB_MODULE_signal-h = true
diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h
index 957d8c6bdb..6cbbb9f869 100644
--- a/nt/inc/ms-w32.h
+++ b/nt/inc/ms-w32.h
@@ -518,8 +518,6 @@ extern int getpagesize (void);
 
 extern void * memrchr (void const *, int, size_t);
 
-extern int mkostemp (char *, int);
-
 
 #if defined (__MINGW32__)
 
diff --git a/nt/mingw-cfg.site b/nt/mingw-cfg.site
index a106717979..d9a824008c 100644
--- a/nt/mingw-cfg.site
+++ b/nt/mingw-cfg.site
@@ -79,7 +79,6 @@ ac_cv_func_getaddrinfo=yes
 # Implemented as an inline function in ws2tcpip.h
 ac_cv_func_gai_strerror=yes
 # Implemented in w32.c
-ac_cv_func_mkostemp=yes
 ac_cv_func_readlink=yes
 ac_cv_func_symlink=yes
 # Avoid run-time tests of readlink and symlink, which will fail
diff --git a/src/w32.c b/src/w32.c
index fa3cbe183f..1c58f92d57 100644
--- a/src/w32.c
+++ b/src/w32.c
@@ -4397,61 +4397,6 @@ sys_open (const char * path, int oflag, int mode)
   return res;
 }
 
-/* Implementation of mkostemp for MS-Windows, to avoid race conditions
-   when using mktemp.
-
-   Standard algorithm for generating a temporary file name seems to be
-   use pid or tid with a letter on the front (in place of the 6 X's)
-   and cycle through the letters to find a unique name.  We extend
-   that to allow any reasonable character as the first of the 6 X's,
-   so that the number of simultaneously used temporary files will be
-   greater.  */
-
-int
-mkostemp (char * template, int flags)
-{
-  char * p;
-  int i, fd = -1;
-  unsigned uid = GetCurrentThreadId ();
-  int save_errno = errno;
-  static char first_char[] = "abcdefghijklmnopqrstuvwyz0123456789!%-_@#";
-
-  errno = EINVAL;
-  if (template == NULL)
-    return -1;
-
-  p = template + strlen (template);
-  i = 5;
-  /* replace up to the last 5 X's with uid in decimal */
-  while (--p >= template && p[0] == 'X' && --i >= 0)
-    {
-      p[0] = '0' + uid % 10;
-      uid /= 10;
-    }
-
-  if (i < 0 && p[0] == 'X')
-    {
-      i = 0;
-      do
-	{
-	  p[0] = first_char[i];
-	  if ((fd = sys_open (template,
-			      flags | _O_CREAT | _O_EXCL | _O_RDWR,
-			      S_IRUSR | S_IWUSR)) >= 0
-	      || errno != EEXIST)
-	    {
-	      if (fd >= 0)
-		errno = save_errno;
-	      return fd;
-	    }
-	}
-      while (++i < sizeof (first_char));
-    }
-
-  /* Template is badly formed or else we can't generate a unique name.  */
-  return -1;
-}
-
 int
 fchmod (int fd, mode_t mode)
 {
-- 
2.13.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09 23:36       ` Paul Eggert
@ 2017-08-10 15:35         ` Eli Zaretskii
  2017-08-10 22:24           ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-10 15:35 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

> Cc: 28023@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Wed, 9 Aug 2017 16:36:55 -0700
> 
> Eli Zaretskii wrote:
> > Perhaps you are right, and it will "just work".  I will have to try
> > (for now, I just looked at the sources and the changes, but didn't try
> > building).
> 
> It looks to me like it should just work.

Well, it's not that easy after all.  First, nt/gnulib-cfg.mk needs to
be fixed to remove the lines that disable building tempname.c and
mkostmp.c, and nt/mingw-cfg.site needs to remove the line which claims
that mkostmp is available.  Otherwise, tempname.c will not be
compiled.  This is easy to fix.

Then it turns out ms-w32.h only makes its redirections when compiling
with -Demacs, which is not what happens when Gnulib sources are
compiled.  I can fix this, but it will need a few changes in lib-src,
which currently doesn't expect 'open' to be redirected.

And finally, tempname.c calls mkdir with 2 arguments, whereas mkdir on
Windows accepts only one.  (This means Gnulib lacks a dependency,
since the tempname module should then depend on mkdir, which isn't in
lib/.)  Since we don't want the Gnulib replacement for mkdir, we will
have to change w32.c:sys_mkdir to accept 2 arguments to fix this,
unless you have a better idea.

How do you propose to proceed with this?

> You might also consider the second 
> attached patch: it removes the MS-Windows implementation of mkostemp, since the 
> generic code should just work as well.

Once the above problems are taken care of, the mkostemp part should be
easy.

Thanks.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-10 15:35         ` Eli Zaretskii
@ 2017-08-10 22:24           ` Paul Eggert
  2017-08-11  6:21             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-08-10 22:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28023

Eli Zaretskii wrote:


> And finally, tempname.c calls mkdir with 2 arguments, whereas mkdir on
> Windows accepts only one.  (This means Gnulib lacks a dependency,
> since the tempname module should then depend on mkdir, which isn't in
> lib/.)  Since we don't want the Gnulib replacement for mkdir, we will
> have to change w32.c:sys_mkdir to accept 2 arguments to fix this,
> unless you have a better idea.

No, your ideas all sound good. It's simpler for the generic code, if the mkdir 
replacement acts like POSIX mkdir.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-10 22:24           ` Paul Eggert
@ 2017-08-11  6:21             ` Eli Zaretskii
  2017-08-11  7:44               ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-11  6:21 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

> Cc: 28023@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Thu, 10 Aug 2017 15:24:13 -0700
> 
> No, your ideas all sound good. It's simpler for the generic code, if the mkdir 
> replacement acts like POSIX mkdir.

So how to proceed?  Do you want to wait until I make all those changes
on master?  Or push a branch with the changes?  Something else?





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-11  6:21             ` Eli Zaretskii
@ 2017-08-11  7:44               ` Paul Eggert
  2017-08-11  8:03                 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-08-11  7:44 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28023

Eli Zaretskii wrote:
> So how to proceed?  Do you want to wait until I make all those changes
> on master?  Or push a branch with the changes?  Something else?

I'd rather not mess with a branch. If it's easy to make those changes to master 
please do so. If not, I can modify the patch to keep the current algorithm for 
DOS_NT, though this will be ugly.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-11  7:44               ` Paul Eggert
@ 2017-08-11  8:03                 ` Eli Zaretskii
  2017-08-12  8:31                   ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-11  8:03 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

> Cc: 28023@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 11 Aug 2017 00:44:36 -0700
> 
> If it's easy to make those changes to master please do so.

OK, will do.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-11  8:03                 ` Eli Zaretskii
@ 2017-08-12  8:31                   ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-12  8:31 UTC (permalink / raw)
  To: eggert; +Cc: 28023

> Date: Fri, 11 Aug 2017 11:03:04 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 28023@debbugs.gnu.org
> 
> > Cc: 28023@debbugs.gnu.org
> > From: Paul Eggert <eggert@cs.ucla.edu>
> > Date: Fri, 11 Aug 2017 00:44:36 -0700
> > 
> > If it's easy to make those changes to master please do so.
> 
> OK, will do.

Done.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09  9:22   ` Paul Eggert
  2017-08-09 11:21     ` Michael Albinus
@ 2017-08-12 10:35     ` Michael Albinus
  2017-08-12 15:55       ` Paul Eggert
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Albinus @ 2017-08-12 10:35 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

Paul Eggert <eggert@cs.ucla.edu> writes:

Hi Paul,

> Tramp also needs to support the excl flag of write-region (currently
> it ignores that flag).

I've implemented this, commit ec5cfaa456.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 10:35     ` Michael Albinus
@ 2017-08-12 15:55       ` Paul Eggert
  2017-08-12 16:09         ` Eli Zaretskii
  2017-08-12 17:51         ` Michael Albinus
  0 siblings, 2 replies; 23+ messages in thread
From: Paul Eggert @ 2017-08-12 15:55 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 28023

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

Michael Albinus wrote:
> I've implemented this, commit ec5cfaa456.

Thanks. Unfortunately this caused "make check" to fail on Fedora 26. I installed 
the attached patch, which causes the test to pass. Can you please look over it 
to see whether it makes sense, and look for similar problems elsewhere? (I'm not 
that familiar with Tramp.) Thanks.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Adjust-jka-compr-to-recent-Tramp-changes.patch --]
[-- Type: text/x-patch; name="0001-Adjust-jka-compr-to-recent-Tramp-changes.patch", Size: 1703 bytes --]

From bbf52c142afbb9e10bf2ae20b3c77993fda26b43 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 12 Aug 2017 08:52:25 -0700
Subject: [PATCH] Adjust jka-compr to recent Tramp changes.

* lisp/jka-compr.el (jka-compr-write-region):
Two new args LOCKNAME and MUSTBENEW.
---
 lisp/jka-compr.el | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/jka-compr.el b/lisp/jka-compr.el
index 26a7cf5..9e780f8 100644
--- a/lisp/jka-compr.el
+++ b/lisp/jka-compr.el
@@ -252,7 +252,8 @@ jka-compr-make-temp-name
   "This routine will return the name of a new file."
   (make-temp-file jka-compr-temp-name-template))
 
-(defun jka-compr-write-region (start end file &optional append visit)
+(defun jka-compr-write-region (start end file &optional
+                                     append visit lockname mustbenew)
   (let* ((filename (expand-file-name file))
 	 (visit-file (if (stringp visit) (expand-file-name visit) filename))
 	 (info (jka-compr-get-compression-info visit-file))
@@ -334,7 +335,8 @@ jka-compr-write-region
 	      (jka-compr-run-real-handler 'write-region
 					  (list (point-min) (point-max)
 						filename
-						(and append can-append) 'dont))
+						(and append can-append) 'dont
+						lockname mustbenew))
 	      (erase-buffer)) )
 
 	  (delete-file temp-file)
@@ -365,7 +367,8 @@ jka-compr-write-region
 	  nil)
 
       (jka-compr-run-real-handler 'write-region
-				  (list start end filename append visit)))))
+				  (list start end filename append visit
+					lockname mustbenew)))))
 
 
 (defun jka-compr-insert-file-contents (file &optional visit beg end replace)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 15:55       ` Paul Eggert
@ 2017-08-12 16:09         ` Eli Zaretskii
  2017-08-12 16:25           ` Paul Eggert
  2017-08-12 17:51         ` Michael Albinus
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-12 16:09 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023, michael.albinus

Paul,

Looking back at your original patch, I think the function you are
adding should be named make-temp-file-internal, as we do with other
functions whose low-level parts are implemented in C.
fileio--SOMETHING sounds like something we never had, so I think it's
better to a void a new prefix.

Thanks.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 16:09         ` Eli Zaretskii
@ 2017-08-12 16:25           ` Paul Eggert
  2017-08-12 16:52             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-08-12 16:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28023, michael.albinus

Eli Zaretskii wrote:
> I think the function you are
> adding should be named make-temp-file-internal, as we do with other
> functions whose low-level parts are implemented in C.

I was following the lead of names like lread--substitute-command-keys, 
print--preprocess, and thread--blocker, all low-level C functions whose first 
part identifies which C module they're in. Although I see that the "-internal" 
suffix is more popular for this sort of thing, isn't that a revenant of the old 
days, before we instituted the convention of using PREFIX--NAME for private 
names? Or is the "-internal" suffix a separate naming convention, used by both 
Lisp and C code, that has a different semantics from PREFIX--NAME? If so, it 
would be nice to have advice somewhere as to when to use the -internal suffix vs 
when to use PREFIX--NAME.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 16:25           ` Paul Eggert
@ 2017-08-12 16:52             ` Eli Zaretskii
  2017-08-12 17:57               ` Paul Eggert
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-12 16:52 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023, michael.albinus

> Cc: michael.albinus@gmx.de, 28023@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 12 Aug 2017 09:25:37 -0700
> 
> I was following the lead of names like lread--substitute-command-keys, 
> print--preprocess, and thread--blocker, all low-level C functions whose first 
> part identifies which C module they're in. Although I see that the "-internal" 
> suffix is more popular for this sort of thing, isn't that a revenant of the old 
> days, before we instituted the convention of using PREFIX--NAME for private 
> names? Or is the "-internal" suffix a separate naming convention, used by both 
> Lisp and C code, that has a different semantics from PREFIX--NAME? If so, it 
> would be nice to have advice somewhere as to when to use the -internal suffix vs 
> when to use PREFIX--NAME.

I think the prefix that comes from the file where the function is
defined is more of a Lisp convention, and the -internal convention is
more for C implementations.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 15:55       ` Paul Eggert
  2017-08-12 16:09         ` Eli Zaretskii
@ 2017-08-12 17:51         ` Michael Albinus
  1 sibling, 0 replies; 23+ messages in thread
From: Michael Albinus @ 2017-08-12 17:51 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023

Paul Eggert <eggert@cs.ucla.edu> writes:

Hi Paul,

> Thanks. Unfortunately this caused "make check" to fail on Fedora 26. I
> installed the attached patch, which causes the test to pass. Can you
> please look over it to see whether it makes sense, and look for
> similar problems elsewhere? (I'm not that familiar with Tramp.)

The patch is OK. Sorry for the trouble, I've searched for
"-write-region" in the whole lisp directory for such cases, but I must
have missed jka-compr.el.

> Thanks.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 16:52             ` Eli Zaretskii
@ 2017-08-12 17:57               ` Paul Eggert
  2017-08-12 18:07                 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Eggert @ 2017-08-12 17:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 28023, michael.albinus

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

Eli Zaretskii wrote:
> I think the prefix that comes from the file where the function is
> defined is more of a Lisp convention, and the -internal convention is
> more for C implementations.

I see lots of exceptions to these conventions, presumably either because 
developers don't know the intent or because the code was written before the 
conventions were established. Still, it's better to document the intent so I 
installed the attached patch to do that. I'll adjust the make-temp-file patch 
accordingly.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Document-internal-use-naming-conventions.patch --]
[-- Type: text/x-patch; name="0001-Document-internal-use-naming-conventions.patch", Size: 2247 bytes --]

From 57c2de6fce73b77081e59d0527b12f158bac758a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 12 Aug 2017 10:54:32 -0700
Subject: [PATCH] Document internal-use naming conventions

* doc/lispref/functions.texi (Function Names):
* doc/lispref/variables.texi (Tips for Defining):
Document naming conventions for internal-use functions and vars.
See Bug#28023#59.
---
 doc/lispref/functions.texi |  9 +++++++++
 doc/lispref/variables.texi | 10 ++++++++++
 2 files changed, 19 insertions(+)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index 283f74f..06de2e2 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -544,6 +544,15 @@ Function Names
 conflict.  (This is not the case in some dialects of Lisp, like
 Scheme.)
 
+  By convention, if a function's symbol consists of two names
+separated by @samp{--}, the function is intended for internal use and
+the first part names the file defining the function.  For example, a
+function named @code{vc-git--rev-parse} is an internal function
+defined in @file{vc-git.el}.  Internal-use functions written in C have
+names ending in @samp{-internal}, e.g., @code{bury-buffer-internal}.
+Emacs code contributed before 2018 may follow other internal-use
+naming conventions, which are being phased out.
+
 @node Defining Functions
 @section Defining Functions
 @cindex defining a function
diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi
index 2818ea0..7650ed4 100644
--- a/doc/lispref/variables.texi
+++ b/doc/lispref/variables.texi
@@ -574,6 +574,16 @@ Tips for Defining
 
 @item @dots{}-switches
 The value specifies options for a command.
+
+@item @var{prefix}--@dots{}
+The variable is intended for internal use and is defined in the file
+@file{@var{prefix}.el}.  (Emacs code contributed before 2018 may
+follow other conventions, which are being phased out.)
+
+@item @dots{}-internal
+The variable is intended for internal use and is defined in C code.
+(Emacs code contributed before 2018 may follow other conventions,
+which are being phased out.)
 @end table
 
   When you define a variable, always consider whether you should mark
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-12 17:57               ` Paul Eggert
@ 2017-08-12 18:07                 ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2017-08-12 18:07 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 28023, michael.albinus

> Cc: michael.albinus@gmx.de, 28023@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 12 Aug 2017 10:57:26 -0700
> 
> I see lots of exceptions to these conventions, presumably either because 
> developers don't know the intent or because the code was written before the 
> conventions were established. Still, it's better to document the intent so I 
> installed the attached patch to do that.

Thanks, I agree this should be documented.

> I'll adjust the make-temp-file patch accordingly.

Thanks.





^ permalink raw reply	[flat|nested] 23+ messages in thread

* bug#28023: fix make-temp-file race on local host
  2017-08-09 11:21     ` Michael Albinus
@ 2017-08-13  7:21       ` Paul Eggert
  0 siblings, 0 replies; 23+ messages in thread
From: Paul Eggert @ 2017-08-13  7:21 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 28023-done

Michael Albinus wrote:
> Paul Eggert<eggert@cs.ucla.edu>  writes:
> 
>> Tramp needs to support a new method make-temp-file that creates a file
>> (or directory) atomically, as make-temp-file does locally now.
> That means, make-temp-file shall be converted into a magic file name
> operation. I'll do my best, but I don't know whether we could get an
> implementation for all Tramp methods w/o a race condition.
> 
>> Tramp also needs to support the excl flag of write-region (currently
>> it ignores that flag).
> This sounds trivial. And yes, it will help.

Thanks for doing that. Because of that, I'm no longer seeing a race in 
make-temp-file, even for Tramp files. That is, although my proposed patch is 
still a performance win on local files, I don't see how it is a correctness win 
any more. It's still worth installing for the performance reasons, though, so I 
did that and I am marking this bug as done.

>> I was planning to write this up as a bug report after the patch goes
>> in.
> Pls do.

To some extent this is moot now, if I understand things correctly. That is, the 
only reason to write this up now would be for performance reasons, not a race 
condition.

There is still a race involving destination directories, for both Tramp and 
non-Tramp versions. I plan to take a look at that next.





^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2017-08-13  7:21 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-09  5:38 bug#28023: fix make-temp-file race on local host Paul Eggert
2017-08-09  6:51 ` Michael Albinus
2017-08-09  9:22   ` Paul Eggert
2017-08-09 11:21     ` Michael Albinus
2017-08-13  7:21       ` Paul Eggert
2017-08-12 10:35     ` Michael Albinus
2017-08-12 15:55       ` Paul Eggert
2017-08-12 16:09         ` Eli Zaretskii
2017-08-12 16:25           ` Paul Eggert
2017-08-12 16:52             ` Eli Zaretskii
2017-08-12 17:57               ` Paul Eggert
2017-08-12 18:07                 ` Eli Zaretskii
2017-08-12 17:51         ` Michael Albinus
2017-08-09 16:05 ` Eli Zaretskii
2017-08-09 18:47   ` Paul Eggert
2017-08-09 19:09     ` Eli Zaretskii
2017-08-09 23:36       ` Paul Eggert
2017-08-10 15:35         ` Eli Zaretskii
2017-08-10 22:24           ` Paul Eggert
2017-08-11  6:21             ` Eli Zaretskii
2017-08-11  7:44               ` Paul Eggert
2017-08-11  8:03                 ` Eli Zaretskii
2017-08-12  8:31                   ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).