5 files changed, 158 insertions, 86 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index 1b8b3c56004..adff00f7189 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,51 @@
+2013-03-02  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.
+        (lock_file): Don't access freed storage.
+
+2013-03-02  Andreas Schwab  <schwab@linux-m68k.org>
+
+        * lisp.h (XPNTR) [!USE_LSB_TAG]: Remove extra paren.  (Bug#13734)
+
+2013-03-02  Paul Eggert  <eggert@cs.ucla.edu>
+
+        * textprop.c: Use bool for booleans.
+        (validate_interval_range, Fadd_text_properties)
+        (Fremove_text_properties): Prefer bool to int when either works.
+
+2013-03-02  Eli Zaretskii  <eliz@gnu.org>
+
+        * textprop.c (Fadd_text_properties, Fremove_text_properties): If
+        the interval tree changes as a side effect of calling
+        modify_region, re-do processing starting from the call to
+        validate_interval_range.  (Bug#13743)
+
 2013-02-28  Eli Zaretskii  <eliz@gnu.org>
 
         * w32.c (sys_open): Don't reset the flags for FD in fd_info[].
diff --git a/src/filelock.c b/src/filelock.c
index 78cd60a12e1..14b9d4aaca5 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -64,7 +64,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #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.
 
@@ -97,7 +98,12 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    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.  */
 
 
 /* Return the time of the last system boot.  */
@@ -291,55 +297,31 @@ typedef struct
 /* Free the two dynamically-allocated pieces in PTR.  */
 #define FREE_LOCK_INFO(i) do { xfree ((i).user); xfree ((i).host); } while (0)
 
-
-/* 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)
+enum { defined_WINDOWSNT = 1 };
 #else
-#define IS_LOCK_FILE(ST) S_ISLNK ((ST).st_mode)
+enum { defined_WINDOWSNT = 0 };
 #endif
 
+/* Write the name of the lock file for FNAME into LOCKNAME.  Length
+   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
@@ -356,8 +338,8 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
   {
     /* Deny everybody else any kind of access to the file until we are
        done writing it and close the handle.  This makes the entire
-       open/write/close operation atomic, as far as other processes
-       are concerned.  */
+       open/write/close operation atomic, as far as other WINDOWSNT
+       processes are concerned.  */
     int fd = _sopen (lfname,
                      _O_WRONLY | _O_BINARY | _O_CREAT | _O_EXCL | _O_NOINHERIT,
                      _SH_DENYRW, S_IREAD | S_IWRITE);
@@ -380,7 +362,7 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
   }
 #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);
@@ -440,6 +422,8 @@ read_lock_data (char *lfname)
 #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)
@@ -601,6 +585,7 @@ lock_if_free (lock_info_type *clasher, register char *lfname)
    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
@@ -612,11 +597,9 @@ lock_if_free (lock_info_type *clasher, register char *lfname)
 void
 lock_file (Lisp_Object fn)
 {
-  register Lisp_Object attack, orig_fn, encoded_fn;
-  register char *lfname, *locker;
-  ptrdiff_t locker_size;
+  Lisp_Object orig_fn, encoded_fn;
+  char *lfname;
   lock_info_type lock_info;
-  printmax_t pid;
   struct gcpro gcpro1;
   USE_SAFE_ALLOCA;
 
@@ -657,38 +640,36 @@ lock_file (Lisp_Object fn)
       call1 (intern ("ask-user-about-supersession-threat"), fn);
 
   }
-  UNGCPRO;
-
-  /* Try to lock the lock. */
-  if (lock_if_free (&lock_info, lfname) <= 0)
-    /* Return now if we have locked it, or if lock creation failed */
-    return;
 
-  /* Else consider breaking the lock */
-  locker_size = (strlen (lock_info.user) + strlen (lock_info.host)
-                 + INT_STRLEN_BOUND (printmax_t)
-                 + sizeof "@ (pid )");
-  locker = SAFE_ALLOCA (locker_size);
-  pid = lock_info.pid;
-  esprintf (locker, "%s@%s (pid %"pMd")",
-            lock_info.user, lock_info.host, pid);
-  FREE_LOCK_INFO (lock_info);
-
-  attack = call2 (intern ("ask-user-about-lock"), fn, build_string (locker));
-  SAFE_FREE ();
-  if (!NILP (attack))
-    /* User says take the lock */
+  /* Try to lock the lock.  */
+  if (0 < lock_if_free (&lock_info, lfname))
     {
-      lock_file_1 (lfname, 1);
-      return;
+      /* Someone else has the lock.  Consider breaking it.  */
+      ptrdiff_t locker_size = (strlen (lock_info.user) + strlen (lock_info.host)
+                               + INT_STRLEN_BOUND (printmax_t)
+                               + sizeof "@ (pid )");
+      char *locker = SAFE_ALLOCA (locker_size);
+      printmax_t pid = lock_info.pid;
+      Lisp_Object attack;
+      esprintf (locker, "%s@%s (pid %"pMd")",
+                lock_info.user, lock_info.host, pid);
+      FREE_LOCK_INFO (lock_info);
+
+      attack = call2 (intern ("ask-user-about-lock"), fn, build_string (locker));
+      /* Take the lock if the user said so.  */
+      if (!NILP (attack))
+        lock_file_1 (lfname, 1);
     }
-  /* User says ignore the lock */
+
+  UNGCPRO;
+  SAFE_FREE ();
 }
 
 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);
@@ -697,6 +678,8 @@ unlock_file (register Lisp_Object fn)
 
   if (current_lock_owner (0, lfname) == 2)
     unlink (lfname);
+
+  SAFE_FREE ();
 }
 
 void
@@ -762,9 +745,10 @@ t if it is locked by you, else a string saying which user has locked it.  */)
   (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);
 
@@ -781,6 +765,7 @@ t if it is locked by you, else a string saying which user has locked it.  */)
   if (owner > 0)
     FREE_LOCK_INFO (locker);
 
+  SAFE_FREE ();
   return ret;
 }
 
diff --git a/src/intervals.h b/src/intervals.h
index d6191225b1f..a38e83cf10e 100644
--- a/src/intervals.h
+++ b/src/intervals.h
@@ -259,7 +259,7 @@ extern Lisp_Object get_local_map (ptrdiff_t, struct buffer *, Lisp_Object);
 extern INTERVAL update_interval (INTERVAL, ptrdiff_t);
 extern void set_intervals_multibyte (bool);
 extern INTERVAL validate_interval_range (Lisp_Object, Lisp_Object *,
-                                         Lisp_Object *, int);
+                                         Lisp_Object *, bool);
 extern INTERVAL interval_of (ptrdiff_t, Lisp_Object);
 
 /* Defined in xdisp.c.  */
diff --git a/src/lisp.h b/src/lisp.h
index 3d018b2b45e..f978a3b2fe0 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -512,7 +512,7 @@ static EMACS_INT const VALMASK
 
 /* DATA_SEG_BITS forces extra bits to be or'd in with any pointers
    which were stored in a Lisp_Object.  */
-#define XPNTR(a) ((uintptr_t) ((XLI (a) & VALMASK)) | DATA_SEG_BITS))
+#define XPNTR(a) ((uintptr_t) ((XLI (a) & VALMASK) | DATA_SEG_BITS))
 
 #endif /* not USE_LSB_TAG */
 
diff --git a/src/textprop.c b/src/textprop.c
index 9499b53301f..18e893b3ef2 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -125,9 +125,10 @@ modify_region (Lisp_Object buffer, Lisp_Object start, Lisp_Object end)
 #define hard 1
 
 INTERVAL
-validate_interval_range (Lisp_Object object, Lisp_Object *begin, Lisp_Object *end, int force)
+validate_interval_range (Lisp_Object object, Lisp_Object *begin,
+                         Lisp_Object *end, bool force)
 {
-  register INTERVAL i;
+  INTERVAL i;
   ptrdiff_t searchpos;
 
   CHECK_STRING_OR_BUFFER (object);
@@ -1131,6 +1132,7 @@ Return t if any property value actually changed, nil otherwise.  */)
   ptrdiff_t s, len;
   bool modified = 0;
   struct gcpro gcpro1;
+  bool first_time = 1;
 
   properties = validate_plist (properties);
   if (NILP (properties))
@@ -1139,6 +1141,7 @@ Return t if any property value actually changed, nil otherwise.  */)
   if (NILP (object))
     XSETBUFFER (object, current_buffer);
 
+ retry:
   i = validate_interval_range (object, &start, &end, hard);
   if (!i)
     return Qnil;
@@ -1174,8 +1177,25 @@ Return t if any property value actually changed, nil otherwise.  */)
       copy_properties (unchanged, i);
     }
 
-  if (BUFFERP (object))
-    modify_region (object, start, end);
+  if (BUFFERP (object) && first_time)
+    {
+      ptrdiff_t prev_total_length = TOTAL_LENGTH (i);
+      ptrdiff_t prev_pos = i->position;
+
+      modify_region (object, start, end);
+      /* If someone called us recursively as a side effect of
+         modify_region, and changed the intervals behind our back
+         (could happen if lock_file, called by prepare_to_modify_buffer,
+         triggers redisplay, and that calls add-text-properties again
+         in the same buffer), we cannot continue with I, because its
+         data changed.  So we restart the interval analysis anew.  */
+      if (TOTAL_LENGTH (i) != prev_total_length
+          || i->position != prev_pos)
+        {
+          first_time = 0;
+          goto retry;
+        }
+    }
 
   /* We are at the beginning of interval I, with LEN chars to scan.  */
   for (;;)
@@ -1427,10 +1447,12 @@ Use `set-text-properties' if you want to remove all text properties.  */)
   INTERVAL i, unchanged;
   ptrdiff_t s, len;
   bool modified = 0;
+  bool first_time = 1;
 
   if (NILP (object))
     XSETBUFFER (object, current_buffer);
 
+ retry:
   i = validate_interval_range (object, &start, &end, soft);
   if (!i)
     return Qnil;
@@ -1462,8 +1484,25 @@ Use `set-text-properties' if you want to remove all text properties.  */)
       copy_properties (unchanged, i);
     }
 
-  if (BUFFERP (object))
-    modify_region (object, start, end);
+  if (BUFFERP (object) && first_time)
+    {
+      ptrdiff_t prev_total_length = TOTAL_LENGTH (i);
+      ptrdiff_t prev_pos = i->position;
+
+      modify_region (object, start, end);
+      /* If someone called us recursively as a side effect of
+         modify_region, and changed the intervals behind our back
+         (could happen if lock_file, called by prepare_to_modify_buffer,
+         triggers redisplay, and that calls add-text-properties again
+         in the same buffer), we cannot continue with I, because its
+         data changed.  So we restart the interval analysis anew.  */
+      if (TOTAL_LENGTH (i) != prev_total_length
+          || i->position != prev_pos)
+        {
+          first_time = 0;
+          goto retry;
+        }
+    }
 
   /* We are at the beginning of an interval, with len to scan */
   for (;;)