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

From: Paul Eggert <[email-masked]>
Date: Sun, 03 Mar 2013 15:56:03 -0800

On 03/03/2013 08:39 AM, Eli Zaretskii wrote:

> the primitives you used in your suggested patch have problems on Windows:
> 'rename' is not atomic when it needs to delete the target ...
> and hard links are only supported on NTFS

Thanks for mentioning these problems.  The latter issue
affects GNU/Linux, too, since it also can mount FAT32 file
systems.  Attached is a revised patch that tries to address
these problems.

> My testing indicates that 'readdir' does return the file's name, but
> every other system call I tried, including even 'lstat', fails with
> EPERM.  (I don't know whether all NFS servers behave that way.)

They don't.  Traditional NFS servers are stateless, and do
not have a state where a file exists and its parent
directory is accessible but you cannot stat the file.  I'd
even call that behavior a bug: file servers with that
behavior will cause problems with many GNU programs,
including Emacs.  It would not be wise for Emacs to rely on
or encourage that behavior.

> with the changes you installed, I think even if .#FILE
> _can_ be accessed, Emacs on a Posix host will refrain from
> locking it, because 'readlink' will fail for it, right?

That's correct: the existence of a regular-file .#FILE
prevents Emacs from locking FILE, and Emacs goes ahead and
accesses FILE without bothering to lock it.  The assumption
is that .#FILE was created for some other reason and should
not be messed with.

> these issues are unrelated to the MS-Windows build of Emacs.

I don't see why they're unrelated.  If an MS-Windows Emacs
uses regular files for locks, these files get in the way of
GNU/Linux Emacs lock files, and that makes these issues pop up.

> They existed since about forever, and we never cared.
> Why is it suddenly so important that this feature works

It's important because the MS-Windows code was recently
changed to create lock files, the existence of which could
break GNU/Linux Emacs's locking.

> with 110% reliability, something it never did?

Even with the attached patch, lock files are not 100%
reliable.  The main point of my patches have been to prevent
lock files from being significantly less reliable than they
already were.  If we easily can make them more
reliable, so much the better, but that's not the main goal.

=== modified file 'etc/ChangeLog'
--- etc/ChangeLog	2013-03-03 00:02:19 +0000
+++ etc/ChangeLog	2013-03-03 00:19:47 +0000
@@ -1,3 +1,8 @@
+2013-03-03  Paul Eggert  <[email-masked]>
+
+	FILE's lock is now always .#FILE and may be a regular file (Bug#13807).
+	* NEWS: Document this.
+
 2013-03-02  Bill Wohler  <[email-masked]>
 
 	Release MH-E version 8.5.

=== modified file 'etc/NEWS'
--- etc/NEWS	2013-03-03 00:02:19 +0000
+++ etc/NEWS	2013-03-03 00:18:13 +0000
@@ -320,7 +320,7 @@
 ** 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'.
+** The lock for 'DIR/FILE' is now 'DIR/.#FILE' and may be a regular 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
@@ -328,9 +328,8 @@
 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.
+On file systems that do not support symbolic links, the lock is now a
+regular file with contents being what would have been in the symlink.
 
 ** 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

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-03-02 21:05:52 +0000
+++ src/ChangeLog	2013-03-03 23:12:54 +0000
@@ -1,5 +1,57 @@
+2013-03-03  Paul Eggert  <[email-masked]>
+
+	FILE's lock is now always .#FILE and may be a regular file (Bug#13807).
+	* filelock.c: Include <c-ctype.h>.
+	(MAX_LFINFO): New top-level constant.
+	(lock_info_type): Remove members pid, boot_time.  Add members at,
+	dot, colon.  Change user member to be the entire buffer, not a
+	pointer.  This allows us to handle the case where a foreign
+	pid or boot time exceeds the local range.  All uses changed.
+	(LINKS_MIGHT_NOT_WORK): New constant.
+	(FREE_LOCK_INFO): Remove, as the pieces no longer need freeing.
+	(defined_WINDOWSNT): Remove.
+	(MAKE_LOCK_NAME, file_in_lock_file_name):
+	Always use .#FILE (not .#-FILE) for the file lock,
+	even if it is a regular file.
+	(rename_lock_file): New function.
+	(create_lock_file): Use it.
+	(create_lock_file, read_lock_data):
+	Prefer a symbolic link for the lock file, falling back on a
+	regular file if symlinks don't work.  Do not try to create
+	symlinks on MS-Windows, due to security hassles.  Stick with
+	POSIXish functions (open, read, write, close, readlink, symlink,
+	link, rename, unlink, mkstemp) when creating locks, as a GNUish
+	host may be using a Windowsish file system, and cannot use
+	MS-Windows-only system calls.  Fall back on mktemp if mkstemp
+	doesn't work.  Don't fail merely because of a symlink-contents
+	length limit in the current file system; fall back on regular
+	files.  Increase the symlink contents length limit to 8 KiB, this
+	should be big enough for any real use and doesn't crunch the
+	stack.
+	(create_lock_file, lock_file_1, read_lock_data):
+	Simplify allocation of lock file buffers now that they fit in 8 KiB.
+	(lock_file_1): Return error number, not bool.  All callers changed.
+	(read_lock_data): Return size of lock file contents, not Lisp object.
+	All callers changed.  Check the contents of a regular-file lock
+	for null bytes.  Handle a race condition if some other process
+	replaces a regular-file lock with a symlink lock or vice versa,
+	while we're trying to read the lock.
+	(current_lock_owner): Parse contents more carefully, to help avoid
+	confusing a regular-file lock with some other application's use
+	of the file.  Check for lock file contents being too long, or
+	not parsing correctly.
+	(current_lock_owner, lock_file):
+	Allow foreign pid and boot times that exceed the local range.
+	(current_lock_owner, lock_if_free, lock_file):
+	Simplify allocation of lock file contents.
+	* w32.c (sys_rename_replace): New function, containing most of
+	the contents of the old sys_rename.
+	(sys_rename): Use it.
+	* w32.h (sys_rename_replace): New decl.
+
 2013-03-02  Paul Eggert  <[email-masked]>
 
+
 	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.

=== modified file 'src/filelock.c'
--- src/filelock.c	2013-03-02 21:05:52 +0000
+++ src/filelock.c	2013-03-03 23:35:40 +0000
@@ -38,6 +38,8 @@
 
 #include <errno.h>
 
+#include <c-ctype.h>
+
 #include "lisp.h"
 #include "character.h"
 #include "buffer.h"
@@ -64,7 +66,7 @@
 #define WTMP_FILE "/var/log/wtmp"
 #endif
 
-/* On non-MS-Windows systems, use a symbolic link to represent a lock.
+/* Normally 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 [email-masked]'.  This avoids a single
    mount (== failure) point for lock files.
@@ -100,10 +102,21 @@
 
    [email-masked]/[email-masked].
 
-   On MS-Windows, symbolic links do not work well, so instead of a
-   symlink .#FN -> [email-masked]', the lock is a regular file .#-FN
-   with contents [email-masked]'.  MS-Windows and non-MS-Windows
-   versions of Emacs ignore each other's locks.  */
+   On some file systems, notably those of MS-Windows, symbolic links
+   do not work well, so instead of a symlink .#FN -> [email-masked]',
+   the lock is a regular file .#FN with contents [email-masked]'.  To
+   establish a lock, a nonce file is created and then renamed to .#FN.
+   On MS-Windows this renaming is atomic unless the lock is forcibly
+   acquired.  On other systems the renaming is atomic if the lock is
+   forcibly acquired; if not, the renaming is done via hard links,
+   which is good enough for lock-file purposes.
+
+   To summarize, race conditions can occur with either:
+
+   * Forced locks on MS-Windows systems.
+
+   * Non-forced locks on non-MS-Windows systems that support neither
+     hard nor symbolic links.  */
 
 
 /* Return the time of the last system boot.  */
@@ -284,30 +297,31 @@
 }
 #endif /* BOOT_TIME */
 
+/* An arbitrary limit on lock contents length.  8 K should be plenty
+   big enough in practice.  */
+enum { MAX_LFINFO = 8 * 1024 };
+
 /* Here is the structure that stores information about a lock.  */
 
 typedef struct
 {
-  char *user;
-  char *host;
-  pid_t pid;
-  time_t boot_time;
+  /* Location of '@', '.', ':' in USER.  If there's no colon, COLON
+     points to the end of USER.  */
+  char *at, *dot, *colon;
+
+  /* Lock file contents [email-masked] with an optional :BOOT_TIME
+     appended.  This memory is used as a lock file contents buffer, so
+     it needs room for MAX_LFINFO + 1 bytes.  A string " (pid NNNN)"
+     may be appended to the USER@HOST while generating a diagnostic,
+     so make room for its extra bytes (as opposed to ".NNNN") too.  */
+  char user[MAX_LFINFO + 1 + sizeof " (pid )" - sizeof "."];
 } lock_info_type;
 
-/* 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 FNAME plus two more for the leading ".#",
-   plus one for "-" if MS-Windows, plus one for the null.  */
+   will be that of FNAME plus two more for the leading ".#", plus one
+   for the null.  */
 #define MAKE_LOCK_NAME(lockname, fname) \
-  (lockname = SAFE_ALLOCA (SBYTES (fname) + 2 + defined_WINDOWSNT + 1), \
+  (lockname = SAFE_ALLOCA (SBYTES (fname) + 2 + 1), \
    fill_in_lock_file_name (lockname, fname))
 
 static void
@@ -319,70 +333,121 @@
   memcpy (lockfile, SSDATA (fn), dirlen);
   lockfile[dirlen] = '.';
   lockfile[dirlen + 1] = '#';
-  if (defined_WINDOWSNT)
-    lockfile[dirlen + 2] = '-';
-  strcpy (lockfile + dirlen + 2 + defined_WINDOWSNT, base);
-}
+  strcpy (lockfile + dirlen + 2, base);
+}
+
+/* For some reason Linux kernels return EPERM on file systems that do
+   not support hard or symbolic links.  This symbol documents the quirk.
+   There is no way to tell whether a symlink call fails due to
+   permissions issues or because links are not supported, but luckily
+   the lock file code should work either way.  */
+enum { LINKS_MIGHT_NOT_WORK = EPERM };
+
+/* Rename OLD to NEW.  If FORCE, replace any existing NEW.
+   It is OK if there are temporarily two hard links to OLD.
+   Return 0 if successful, -1 (setting errno) otherwise.  */
+static int
+rename_lock_file (char const *old, char const *new, bool force)
+{
+#ifdef WINDOWSNT
+  return sys_rename_replace (old, new, force);
+#else
+  if (! force)
+    {
+      if (link (old, new) == 0)
+	return unlink (old) == 0 || errno == ENOENT ? 0 : -1;
+      if (errno != ENOSYS && errno != LINKS_MIGHT_NOT_WORK)
+	return -1;
+
+      /* 'link' does not work on this file system.  This can occur on
+	 a GNU/Linux host mounting a FAT32 file system.  Fall back on
+	 'rename' after checking that NEW does not exist.  There is a
+	 potential race condition since some other process may create
+	 NEW immediately after the existence check, but it's the best
+	 we can portably do here.  */
+      if (check_existing (new))
+	{
+	  errno = EEXIST;
+	  return -1;
+	}
+    }
+
+  return rename (old, new);
+#endif
+}
+
+/* Create the lock file FILE with contents CONTENTS.  Return 0 if
+   successful, an errno value on failure.  If FORCE, remove any
+   existing FILE if necessary.  */
 
 static int
 create_lock_file (char *lfname, char *lock_info_str, bool force)
 {
-  int err;
-
 #ifdef WINDOWSNT
-  /* Symlinks are supported only by latest versions of Windows, and
-     creating them is a privileged operation that often triggers UAC
-     elevation prompts.  Therefore, instead of using symlinks, we
-     create a regular file with the lock info written as its
-     contents.  */
-  {
-    /* 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 WINDOWSNT
-       processes are concerned.  */
-    int fd = _sopen (lfname,
-		     _O_WRONLY | _O_BINARY | _O_CREAT | _O_EXCL | _O_NOINHERIT,
-		     _SH_DENYRW, S_IREAD | S_IWRITE);
-
-    if (fd < 0 && errno == EEXIST && force)
-      fd = _sopen (lfname, _O_WRONLY | _O_BINARY | _O_TRUNC |_O_NOINHERIT,
-		   _SH_DENYRW, S_IREAD | S_IWRITE);
-    if (fd >= 0)
-      {
-	ssize_t lock_info_len = strlen (lock_info_str);
-
-	err = 0;
-	if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len)
-	  err = -1;
-	if (emacs_close (fd))
-	  err = -1;
-      }
-    else
-      err = -1;
-  }
+  /* Symlinks are supported only by later versions of Windows, and
+     creating them is a privileged operation that often triggers
+     User Account Control elevation prompts.  Avoid the problem by
+     pretending that 'symlink' does not work.  */
+  int err = ENOSYS;
 #else
-  err = symlink (lock_info_str, lfname);
-  if (err != 0 && errno == EEXIST && force)
+  int err = symlink (lock_info_str, lfname) == 0 ? 0 : errno;
+#endif
+
+  if (err == EEXIST && force)
     {
       unlink (lfname);
-      err = symlink (lock_info_str, lfname);
+      err = symlink (lock_info_str, lfname) == 0 ? 0 : errno;
     }
+
+  if (err == ENOSYS || err == LINKS_MIGHT_NOT_WORK || err == ENAMETOOLONG)
+    {
+      static char const nonce_base[] = ".#-emacsXXXXXX";
+      char *last_slash = strrchr (lfname, '/');
+      ptrdiff_t lfdirlen = last_slash + 1 - lfname;
+      USE_SAFE_ALLOCA;
+      char *nonce = SAFE_ALLOCA (lfdirlen + sizeof nonce_base);
+      int fd;
+      memcpy (nonce, lfname, lfdirlen);
+      strcpy (nonce + lfdirlen, nonce_base);
+
+#if HAVE_MKSTEMP
+      fd = mkstemp (nonce);
+#else
+      mktemp (nonce);
+      fd = emacs_open (nonce, O_WRONLY | O_CREAT | O_EXCL | O_BINARY,
+		       S_IRUSR | S_IWUSR);
 #endif
 
+      if (fd < 0)
+	err = errno;
+      else
+	{
+	  ptrdiff_t lock_info_len = strlen (lock_info_str);
+	  err = 0;
+	  if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
+	      || fchmod (fd, S_IRUSR | S_IRGRP | S_IROTH) != 0)
+	    err = errno;
+	  if (emacs_close (fd) != 0)
+	    err = errno;
+	  if (!err && rename_lock_file (nonce, lfname, force) != 0)
+	    err = errno;
+	  if (err)
+	    unlink (nonce);
+	}
+
+      SAFE_FREE ();
+    }
+
   return err;
 }
 
 /* Lock the lock file named LFNAME.
    If FORCE, do so even if it is already locked.
-   Return true if successful.  */
+   Return 0 if successful, an error number on failure.  */
 
-static bool
+static int
 lock_file_1 (char *lfname, bool force)
 {
-  int err;
-  int symlink_errno;
-  USE_SAFE_ALLOCA;
-
   /* Call this first because it can GC.  */
   printmax_t boot = get_boot_time ();
 
@@ -390,20 +455,16 @@
   char const *user_name = STRINGP (luser_name) ? SSDATA (luser_name) : "";
   Lisp_Object lhost_name = Fsystem_name ();
   char const *host_name = STRINGP (lhost_name) ? SSDATA (lhost_name) : "";
-  ptrdiff_t lock_info_size = (strlen (user_name) + strlen (host_name)
-			      + 2 * INT_STRLEN_BOUND (printmax_t)
-			      + sizeof "@.:");
-  char *lock_info_str = SAFE_ALLOCA (lock_info_size);
+  char lock_info_str[MAX_LFINFO + 1];
   printmax_t pid = getpid ();
 
-  esprintf (lock_info_str, boot ? "%s@%s.%"pMd":%"pMd : "%s@%s.%"pMd,
-	    user_name, host_name, pid, boot);
-  err = create_lock_file (lfname, lock_info_str, force);
+  if (sizeof lock_info_str
+      <= snprintf (lock_info_str, sizeof lock_info_str,
+		   boot ? "%s@%s.%"pMd":%"pMd : "%s@%s.%"pMd,
+		   user_name, host_name, pid, boot))
+    return ENAMETOOLONG;
 
-  symlink_errno = errno;
-  SAFE_FREE ();
-  errno = symlink_errno;
-  return err == 0;
+  return create_lock_file (lfname, lock_info_str, force);
 }
 
 /* Return true if times A and B are no more than one second apart.  */
@@ -414,32 +475,46 @@
   return (a - b >= -1 && a - b <= 1);
 }
 
-static Lisp_Object
-read_lock_data (char *lfname)
+/* Read the data for the lock file LFNAME into LFINFO.  Read at most
+   MAX_LFINFO + 1 bytes.  Return the number of bytes read, or -1
+   (setting errno) on error.  */
+
+static ptrdiff_t
+read_lock_data (char *lfname, char lfinfo[MAX_LFINFO + 1])
 {
-#ifndef WINDOWSNT
-  return emacs_readlinkat (AT_FDCWD, 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)
-    return Qnil;
-
-  nbytes = emacs_read (fd, lfinfo, MAX_LFINFO);
-  emacs_close (fd);
-
-  if (nbytes > 0)
+  ptrdiff_t nbytes;
+
+  while ((nbytes = readlinkat (AT_FDCWD, lfname, lfinfo, MAX_LFINFO + 1)) < 0
+	 && errno == EINVAL)
     {
-      lfinfo[nbytes] = '\0';
-      return build_string (lfinfo);
+      int fd = emacs_open (lfname, O_RDONLY | O_BINARY | O_NOFOLLOW, 0);
+      if (0 <= fd)
+	{
+	  int read_errno;
+	  nbytes = emacs_read (fd, lfinfo, MAX_LFINFO + 1);
+	  read_errno = errno;
+
+	  if (emacs_close (fd) != 0)
+	    return -1;
+	  if (0 < nbytes && memchr (lfinfo, 0, nbytes))
+	    {
+	      errno = EINVAL;
+	      return -1;
+	    }
+	  errno = read_errno;
+	  return nbytes;
+	}
+
+      if (errno != ELOOP)
+	return -1;
+
+      /* readlinkat saw a non-symlink, but emacs_open saw a symlink.
+	 The former must have been removed and replaced by the latter.
+	 Try again.  */
+      QUIT;
     }
-  else
-    return Qnil;
-#endif
+
+  return nbytes;
 }
 
 /* Return 0 if nobody owns the lock file LFNAME or the lock is obsolete,
@@ -451,83 +526,78 @@
 current_lock_owner (lock_info_type *owner, char *lfname)
 {
   int ret;
-  ptrdiff_t len;
   lock_info_type local_owner;
-  intmax_t n;
-  char *at, *dot, *colon;
-  Lisp_Object lfinfo_object = read_lock_data (lfname);
-  char *lfinfo;
-  struct gcpro gcpro1;
-
-  /* If nonexistent lock file, all is well; otherwise, got strange error. */
-  if (NILP (lfinfo_object))
-    return errno == ENOENT ? 0 : -1;
-  lfinfo = SSDATA (lfinfo_object);
+  ptrdiff_t lfinfolen;
+  intmax_t pid, boot_time;
+  char *at, *dot, *colon, *lfinfo_end;
 
   /* Even if the caller doesn't want the owner info, we still have to
      read it to determine return value.  */
   if (!owner)
     owner = &local_owner;
 
+  /* If nonexistent lock file, all is well; otherwise, got strange error. */
+  lfinfolen = read_lock_data (lfname, owner->user);
+  if (lfinfolen < 0)
+    return errno == ENOENT ? 0 : -1;
+  if (MAX_LFINFO < lfinfolen)
+    return -1;
+  owner->user[lfinfolen] = 0;
+
   /* Parse [email-masked]:BOOT_TIME.  If can't parse, return -1.  */
   /* The USER is everything before the last @.  */
-  at = strrchr (lfinfo, '@');
-  dot = strrchr (lfinfo, '.');
-  if (!at || !dot)
-    return -1;
-  len = at - lfinfo;
-  GCPRO1 (lfinfo_object);
-  owner->user = xmalloc (len + 1);
-  memcpy (owner->user, lfinfo, len);
-  owner->user[len] = 0;
+  owner->at = at = memrchr (owner->user, '@', lfinfolen);
+  if (!at)
+    return -1;
+  owner->dot = dot = strrchr (at, '.');
+  if (!dot)
+    return -1;
 
   /* The PID is everything from the last `.' to the `:'.  */
+  if (! c_isdigit (dot[1]))
+    return -1;
   errno = 0;
-  n = strtoimax (dot + 1, NULL, 10);
-  owner->pid =
-    ((0 <= n && n <= TYPE_MAXIMUM (pid_t)
-      && (TYPE_MAXIMUM (pid_t) < INTMAX_MAX || errno != ERANGE))
-     ? n : 0);
+  pid = strtoimax (dot + 1, &owner->colon, 10);
+  if (errno == ERANGE)
+    pid = -1;
 
-  colon = strchr (dot + 1, ':');
   /* After the `:', if there is one, comes the boot time.  */
-  n = 0;
-  if (colon)
+  switch (owner->colon[0])
     {
-      errno = 0;
-      n = strtoimax (colon + 1, NULL, 10);
+    case 0:
+      boot_time = 0;
+      lfinfo_end = owner->colon;
+      break;
+
+    case ':':
+      if (! c_isdigit (owner->colon[1]))
+	return -1;
+      boot_time = strtoimax (owner->colon + 1, &lfinfo_end, 10);
+      break;
+
+    default:
+      return -1;
     }
-  owner->boot_time =
-    ((0 <= n && n <= TYPE_MAXIMUM (time_t)
-      && (TYPE_MAXIMUM (time_t) < INTMAX_MAX || errno != ERANGE))
-     ? n : 0);
-
-  /* The host is everything in between.  */
-  len = dot - at - 1;
-  owner->host = xmalloc (len + 1);
-  memcpy (owner->host, at + 1, len);
-  owner->host[len] = 0;
-
-  /* We're done looking at the link info.  */
-  UNGCPRO;
+  if (lfinfo_end != owner->user + lfinfolen)
+    return -1;
 
   /* On current host?  */
-  if (STRINGP (Fsystem_name ())
-      && strcmp (owner->host, SSDATA (Fsystem_name ())) == 0)
+  if (STRINGP (Vsystem_name)
+      && dot - (at + 1) == SBYTES (Vsystem_name)
+      && memcmp (at + 1, SSDATA (Vsystem_name), SBYTES (Vsystem_name)) == 0)
     {
-      if (owner->pid == getpid ())
+      if (pid == getpid ())
         ret = 2; /* We own it.  */
-      else if (owner->pid > 0
-               && (kill (owner->pid, 0) >= 0 || errno == EPERM)
-	       && (owner->boot_time == 0
-		   || within_one_second (owner->boot_time, get_boot_time ())))
+      else if (0 < pid && pid <= TYPE_MAXIMUM (pid_t)
+               && (kill (pid, 0) >= 0 || errno == EPERM)
+	       && (boot_time == 0
+		   || (boot_time <= TYPE_MAXIMUM (time_t)
+		       && within_one_second (boot_time, get_boot_time ()))))
         ret = 1; /* An existing process on this machine owns it.  */
-      /* The owner process is dead or has a strange pid (<=0), so try to
+      /* The owner process is dead or has a strange pid, so try to
          zap the lockfile.  */
-      else if (unlink (lfname) < 0)
-        ret = -1;
       else
-	ret = 0;
+        return unlink (lfname);
     }
   else
     { /* If we wanted to support the check for stale locks on remote machines,
@@ -535,11 +605,6 @@
       ret = 1;
     }
 
-  /* Avoid garbage.  */
-  if (owner == &local_owner || ret <= 0)
-    {
-      FREE_LOCK_INFO (*owner);
-    }
   return ret;
 }
 
@@ -551,29 +616,25 @@
    Return -1 if cannot lock for any other reason.  */
 
 static int
-lock_if_free (lock_info_type *clasher, register char *lfname)
+lock_if_free (lock_info_type *clasher, char *lfname)
 {
-  while (! lock_file_1 (lfname, 0))
+  int err;
+  while ((err = lock_file_1 (lfname, 0)) == EEXIST)
     {
-      int locker;
-
-      if (errno != EEXIST)
-	return -1;
-
-      locker = current_lock_owner (clasher, lfname);
-      if (locker == 2)
-        {
-          FREE_LOCK_INFO (*clasher);
-          return 0;   /* We ourselves locked it.  */
-        }
-      else if (locker == 1)
-        return 1;  /* Someone else has it.  */
-      else if (locker == -1)
-	return -1;   /* current_lock_owner returned strange error.  */
+      switch (current_lock_owner (clasher, lfname))
+	{
+	case 2:
+	  return 0;   /* We ourselves locked it.  */
+	case 1:
+	  return 1;   /* Someone else has it.  */
+	case -1:
+	  return -1;  /* current_lock_owner returned strange error.  */
+	}
 
       /* We deleted a stale lock; try again to lock the file.  */
     }
-  return 0;
+
+  return err ? -1 : 0;
 }
 
 /* lock_file locks file FN,
@@ -645,17 +706,16 @@
   if (0 < lock_if_free (&lock_info, lfname))
     {
       /* 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));
+      char *dot = lock_info.dot;
+      ptrdiff_t pidlen = lock_info.colon - (dot + 1);
+      static char const replacement[] = " (pid ";
+      int replacementlen = sizeof replacement - 1;
+      memmove (dot + replacementlen, dot + 1, pidlen);
+      strcpy (dot + replacementlen + pidlen, ")");
+      memcpy (dot, replacement, replacementlen);
+      attack = call2 (intern ("ask-user-about-lock"), fn,
+		      build_string (lock_info.user));
       /* Take the lock if the user said so.  */
       if (!NILP (attack))
 	lock_file_1 (lfname, 1);
@@ -760,10 +820,7 @@
   else if (owner == 2)
     ret = Qt;
   else
-    ret = build_string (locker.user);
-
-  if (owner > 0)
-    FREE_LOCK_INFO (locker);
+    ret = make_string (locker.user, locker.at - locker.user);
 
   SAFE_FREE ();
   return ret;

=== modified file 'src/w32.c'
--- src/w32.c	2013-02-28 06:30:48 +0000
+++ src/w32.c	2013-03-03 23:12:54 +0000
@@ -3416,7 +3416,7 @@
 }
 
 int
-sys_rename (const char * oldname, const char * newname)
+sys_rename_replace (const char *oldname, const char *newname, BOOL force)
 {
   BOOL result;
   char temp[MAX_PATH];
@@ -3472,7 +3472,7 @@
 	return -1;
     }
 
-  /* Emulate Unix behavior - newname is deleted if it already exists
+  /* If FORCE, emulate Unix behavior - newname is deleted if it already exists
      (at least if it is a file; don't do this for directories).
 
      Since we mustn't do this if we are just changing the case of the
@@ -3490,7 +3490,7 @@
 
   result = rename (temp, newname);
 
-  if (result < 0)
+  if (result < 0 && force)
     {
       DWORD w32err = GetLastError ();
 
@@ -3530,6 +3530,12 @@
 }
 
 int
+sys_rename (char const *old, char const *new)
+{
+  return sys_rename_replace (old, new, TRUE);
+}
+
+int
 sys_rmdir (const char * path)
 {
   return _rmdir (map_w32_filename (path, NULL));

=== modified file 'src/w32.h'
--- src/w32.h	2013-02-02 17:14:24 +0000
+++ src/w32.h	2013-03-03 23:12:54 +0000
@@ -186,6 +186,7 @@
 extern void srandom (int);
 extern int random (void);
 
+extern int sys_rename_replace (char const *, char const *, BOOL);
 extern int sys_pipe (int *);
 
 extern void set_process_dir (char *);