bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes

[Paul Eggert <eggert@cs.ucla.edu>]

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

Attached is an updated version of the patch, which avoids
several of the problems mentioned, by using a different lock file name
on MS-Windows.  Non-MS-Windows uses .#FILE symlinks as before;
MS-Windows uses .#-FILE regular files.  This avoids clashes
between the two approaches.  It also means MS-Windows ignores
non-MS-Windows locks and vice versa, but given all the
inherent incompatibilities involved this may be the best that
we can do reliably.

[-- Attachment #2: filelock.txt --]
[-- Type: text/plain, Size: 8307 bytes --]

=== modified file 'etc/ChangeLog'
--- etc/ChangeLog	2013-02-25 17:01:41 +0000
+++ etc/ChangeLog	2013-02-26 21:55:21 +0000
@@ -1,3 +1,7 @@
+2013-02-26  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* NEWS: The lock for FILE is now .#FILE or .#-FILE (Bug#13807).
+
 2013-02-25  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Simplify data_start configuration (Bug#13783).

=== modified file 'etc/NEWS'
--- etc/NEWS	2013-02-25 17:36:03 +0000
+++ etc/NEWS	2013-02-26 21:55:21 +0000
@@ -319,6 +319,18 @@
 ** The `defalias-fset-function' property lets you catch calls to defalias
 and redirect them to your own function instead of `fset'.
 
+** The lock for 'DIR/FILE' is now 'DIR/.#FILE' or 'DIR/.#-FILE'.
+When you edit DIR/FILE, Emacs normally creates a symbolic link
+DIR/.#FILE as a lock that warns other instances of Emacs that DIR/FILE
+is being edited.  Formerly, if there was already a non-symlink file
+named DIR/.#FILE, Emacs fell back on the lock names DIR/.#FILE.0
+through DIR/.#FILE.9.  These fallbacks have been removed, so that
+Emacs now no longer locks DIR/FILE in that case.
+
+On MS-Windows the lock is a regular file DIR/.#-FILE, not a symlink.
+MS-Windows and non-MS-Windows implementations of Emacs ignore each
+other's locks.
+
 ** The 9th element returned by `file-attributes' is now unspecified.
 Formerly, it was t if the file's gid would change if file were deleted
 and recreated.  This value has been inaccurate for years on many

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-02-26 14:28:37 +0000
+++ src/ChangeLog	2013-02-26 21:58:06 +0000
@@ -1,3 +1,33 @@
+2013-02-26  Paul Eggert  <eggert@cs.ucla.edu>
+
+	The lock for FILE is now .#FILE or .#-FILE (Bug#13807).
+	The old approach, which fell back on DIR/.#FILE.0 through
+	DIR/.#FILE.9, had race conditions that could not be easily fixed.
+	If DIR/.#FILE is a non-symlink file, Emacs now does not create a
+	lock file for DIR/FILE; that is, DIR/FILE is no longer partly
+	protected by a lock if DIR/.#FILE is a non-symlink file ("partly"
+	because the locking mechanism was never reliable in that case).
+	This patch fixes this and other bugs discovered by a code
+	inspection that was prompted by
+	<http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.
+	Also, this patch switches to .#-FILE (not .#FILE) on MS-Windows,
+	to avoid interoperability problems between the MS-Windows and
+	non-MS-Windows implementations.  MS-Windows and non-MS-Windows
+	instances of Emacs now ignore each others' locks.
+	* filelock.c (defined_WINDOWSNT): New constant.
+	(MAKE_LOCK_NAME, fill_in_lock_file_name):
+	Don't create DIR/.#FILE.0 through DIR/.#FILE.9.  Instead, create
+	DIR/.#FILE symlinks on non-MS-Windows hosts, and DIR/.#-FILE
+	regular files on MS-Windows hosts.
+	(MAKE_LOCK_NAME, unlock_file, Ffile_locked_p):
+	Use SAFE_ALLOCA to avoid problems with long file names.
+	(MAX_LFINFO): Now a local constant, not a global macro.
+	(IS_LOCK_FILE): Remove.
+	(lock_file_1): Don't inspect errno if symlink call succeeds;
+	that's not portable.
+	(lock_file): Document that this function can return if lock
+	creation fails.
+
 2013-02-26  Bastien Guerry  <bzg@gnu.org>
 
 	* window.c (Frecenter): Tiny docstring enhancement.

=== modified file 'src/filelock.c'
--- src/filelock.c	2013-02-25 17:36:03 +0000
+++ src/filelock.c	2013-02-26 21:49:54 +0000
@@ -63,7 +63,8 @@
 #define WTMP_FILE "/var/log/wtmp"
 #endif
 
-/* The strategy: to lock a file FN, create a symlink .#FN in FN's
+/* On non-MS-Windows systems, use a symbolic link to represent a lock.
+   The strategy: to lock a file FN, create a symlink .#FN in FN's
    directory, with link data `user@host.pid'.  This avoids a single
    mount (== failure) point for lock files.
 
@@ -96,7 +97,13 @@
    has contributed this implementation for Emacs), and was designed by
    Ethan Jacobson, Kimbo Mundy, and others.
 
-   --karl@cs.umb.edu/karl@hq.ileaf.com.  */
+   --karl@cs.umb.edu/karl@hq.ileaf.com.
+
+   On MS-Windows, symbolic links do not work well, so instead of a
+   symlink .#FN -> 'user@host.pid', the lock is a regular file .#-FN
+   with contents 'user@host.pid'.  MS-Windows and non-MS-Windows
+   versions of Emacs ignore each other's locks, and the MS-Windows
+   implementation has races due to its use of non-atomic operations.  */
 
 \f
 /* Return the time of the last system boot.  */
@@ -290,55 +297,31 @@
 /* Free the two dynamically-allocated pieces in PTR.  */
 #define FREE_LOCK_INFO(i) do { xfree ((i).user); xfree ((i).host); } while (0)
 
+#ifdef WINDOWSNT
+enum { defined_WINDOWSNT = 1 };
+#else
+enum { defined_WINDOWSNT = 0 };
+#endif
 
 /* Write the name of the lock file for FNAME into LOCKNAME.  Length
-   will be that of FN plus two more for the leading `.#' plus 1 for
-   the trailing period plus one for the digit after it plus one for
-   the null.  */
-#define MAKE_LOCK_NAME(LOCKNAME, FNAME) \
-  (LOCKNAME = alloca (SBYTES (FNAME) + 2 + 1 + 1 + 1), \
-   fill_in_lock_file_name (LOCKNAME, (FNAME)))
-
-#ifdef WINDOWSNT
-/* 256 chars for user, 1024 chars for host, 10 digits for each of 2 int's.  */
-#define MAX_LFINFO (256 + 1024 + 10 + 10 + 2)
-                                                    /* min size: .@PID */
-#define IS_LOCK_FILE(ST) (MAX_LFINFO >= (ST).st_size && (ST).st_size >= 3)
-#else
-#define IS_LOCK_FILE(ST) S_ISLNK ((ST).st_mode)
-#endif
+   will be that of FNAME plus two more for the leading ".#",
+   plus one for "-" if MS-Windows, plus one for the null.  */
+#define MAKE_LOCK_NAME(lockname, fname) \
+  (lockname = SAFE_ALLOCA (SBYTES (fname) + 2 + defined_WINDOWSNT + 1), \
+   fill_in_lock_file_name (lockname, fname))
 
 static void
-fill_in_lock_file_name (register char *lockfile, register Lisp_Object fn)
+fill_in_lock_file_name (char *lockfile, Lisp_Object fn)
 {
-  ptrdiff_t length = SBYTES (fn);
-  register char *p;
-  struct stat st;
-  int count = 0;
-
-  strcpy (lockfile, SSDATA (fn));
-
-  /* Shift the nondirectory part of the file name (including the null)
-     right two characters.  Here is one of the places where we'd have to
-     do something to support 14-character-max file names.  */
-  for (p = lockfile + length; p != lockfile && *p != '/'; p--)
-    p[2] = *p;
-
-  /* Insert the `.#'.  */
-  p[1] = '.';
-  p[2] = '#';
-
-  p = lockfile + length + 2;
-
-  while (lstat (lockfile, &st) == 0 && !IS_LOCK_FILE (st))
-    {
-      if (count > 9)
-	{
-	  *p = '\0';
-	  return;
-	}
-      sprintf (p, ".%d", count++);
-    }
+  char *last_slash = memrchr (SSDATA (fn), '/', SBYTES (fn));
+  char *base = last_slash + 1;
+  ptrdiff_t dirlen = base - SSDATA (fn);
+  memcpy (lockfile, SSDATA (fn), dirlen);
+  lockfile[dirlen] = '.';
+  lockfile[dirlen + 1] = '#';
+  if (defined_WINDOWSNT)
+    lockfile[dirlen + 2] = '-';
+  strcpy (lockfile + dirlen + 2 + defined_WINDOWSNT, base);
 }
 
 static int
@@ -374,7 +357,7 @@
   }
 #else
   err = symlink (lock_info_str, lfname);
-  if (errno == EEXIST && force)
+  if (err != 0 && errno == EEXIST && force)
     {
       unlink (lfname);
       err = symlink (lock_info_str, lfname);
@@ -434,6 +417,8 @@
 #else
   int fd = emacs_open (lfname, O_RDONLY | O_BINARY, S_IREAD);
   ssize_t nbytes;
+  /* 256 chars for user, 1024 chars for host, 10 digits for each of 2 int's.  */
+  enum { MAX_LFINFO = 256 + 1024 + 10 + 10 + 2 };
   char lfinfo[MAX_LFINFO + 1];
 
   if (fd < 0)
@@ -595,6 +580,7 @@
    decided to go ahead without locking.
 
    When this returns, either the lock is locked for us,
+   or lock creation failed,
    or the user has said to go ahead without locking.
 
    If the file is locked by someone else, this calls
@@ -680,9 +666,10 @@
 }
 
 void
-unlock_file (register Lisp_Object fn)
+unlock_file (Lisp_Object fn)
 {
-  register char *lfname;
+  char *lfname;
+  USE_SAFE_ALLOCA;
 
   fn = Fexpand_file_name (fn, Qnil);
   fn = ENCODE_FILE (fn);
@@ -691,6 +678,8 @@
 
   if (current_lock_owner (0, lfname) == 2)
     unlink (lfname);
+
+  SAFE_FREE ();
 }
 
 void
@@ -756,9 +745,10 @@
   (Lisp_Object filename)
 {
   Lisp_Object ret;
-  register char *lfname;
+  char *lfname;
   int owner;
   lock_info_type locker;
+  USE_SAFE_ALLOCA;
 
   filename = Fexpand_file_name (filename, Qnil);
 
@@ -775,6 +765,7 @@
   if (owner > 0)
     FREE_LOCK_INFO (locker);
 
+  SAFE_FREE ();
   return ret;
 }