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: 1220 bytes --]

On 03/02/2013 01:17 PM, Eli Zaretskii wrote:
> Can you describe your testing
> in more details, and what versions of NFS and Windows did you use?

Sure, I created a MS-Windows style lock file .#FILE by hand, then
edited FILE with a GNU/Linux Emacs.  I didn't build the MS-Windows
Emacs, and didn't need to use NFS to reproduce the problem.

There's another issue: a GNU/Linux Emacs might be using an
MS-Windows file system that does not support symbolic links.  Such
an Emacs should use a regular-file lock, just as MS-Windows Emacs does,
which means that the code to create regular-file locks should be implementable
in POSIXish primitives.  Also, locking should work even if the Emacs
instance that created a lock uses a symlink whereas the Emacs instance
trying to get the lock would use a regular file, or vice versa.

It'll take some thinking to get all this to work well.  I've written
a first cut for this and have attached it.  I have not tested this
on MS-Windows at all, and haven't tested it as much as I'd like on
GNU/Linux, but it should give a feel for the sort of changes that
need to be made.  Unfortunately the patch is a bit complicated,
but to some extent this is inherent in such a complicated area.

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

=== modified file 'etc/ChangeLog'
--- etc/ChangeLog	2013-03-02 20:41:53 +0000
+++ etc/ChangeLog	2013-03-02 22:29:23 +0000
@@ -1,5 +1,8 @@
 2013-03-02  Paul Eggert  <eggert@cs.ucla.edu>
 
+	The lock for FILE is now .#FILE and may be a regular file (Bug#13807).
+	* NEWS: Document this.
+
 	* NEWS: The lock for FILE is now .#FILE or .#-FILE (Bug#13807).
 
 2013-03-01  Michael Albinus  <michael.albinus@gmx.de>

=== modified file 'etc/NEWS'
--- etc/NEWS	2013-03-02 20:41:53 +0000
+++ etc/NEWS	2013-03-02 22:29:23 +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-02 22:29:23 +0000
@@ -1,5 +1,43 @@
 2013-03-02  Paul Eggert  <eggert@cs.ucla.edu>
 
+	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 host member.  Add members at, dot.
+	Change user member to be the entire buffer, not a pointer.
+	(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.
+	(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_if_free, lock_file): Simplify allocation
+	of lock file contents.
+
 	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-02 22:29:23 +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 `user@host.pid'.  This avoids a single
    mount (== failure) point for lock files.
@@ -100,10 +102,11 @@
 
    --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.  */
+   On some file systems, notably those of 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'.  The
+   lock is created atomically by creating a nonce file and hard-linking
+   the nonce to .#FN; the link operation establishes the lock.  .*/
 
 \f
 /* Return the time of the last system boot.  */
@@ -284,30 +287,33 @@
 }
 #endif /* BOOT_TIME */
 \f
+/* 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;
+  /* Location of '@' and '.' in USER.  */
+  char *at, *dot;
+
   pid_t pid;
   time_t boot_time;
+
+  /* Lock file contents USER@HOST.PID 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 that too.  */
+  char user[MAX_LFINFO + INT_STRLEN_BOUND (printmax_t) + sizeof "@ (pid )"];
 } 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 +325,98 @@
   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);
 }
 
+/* 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
+  /* Symlinks are supported only by later 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;
-  }
+     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;
+    }
+
+  /* For some reason Linux kernels return EPERM on file systems
+     that do not support symbolic links.  Turn this into ENOSYS.  */
+  if (err == EPERM)
+    {
+      char *last_slash = strrchr (lfname, '/');
+      char ch = last_slash[1];
+      last_slash[1] = 0;
+      if (faccessat (AT_FDCWD, lfname, W_OK | X_OK, AT_EACCESS) == 0)
+	err = ENOSYS;
+      last_slash[1] = ch;
+    }
+
+  if (err == ENOSYS || 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;
+
+	  /* Rename NONCE to LFNAME, but do not replace any existing
+	     LFNAME unless FORCE.  It is OK if there are temporarily
+	     two hard links to LFNAME.  */
+	  if (!err && (force ? rename : link) (nonce, lfname) != 0)
+	    err = errno;
+	  if ((!force || err) && unlink (nonce) != 0
+	      && !err && errno != ENOENT)
+	    err = errno;
+	}
+
+      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 +424,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 +444,46 @@
   return (a - b >= -1 && a - b <= 1);
 }
 \f
-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 +495,84 @@
 current_lock_owner (lock_info_type *owner, char *lfname)
 {
   int ret;
-  ptrdiff_t len;
   lock_info_type local_owner;
+  ptrdiff_t lfinfolen;
+  pid_t pid;
+  time_t boot_time;
   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);
+  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 USER@HOST.PID: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;
+  at = memrchr (owner->user, '@', lfinfolen);
+  if (!at)
+    return -1;
+  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);
+  n = strtoimax (dot + 1, &colon, 10);
+  if (! (n <= TYPE_MAXIMUM (pid_t)
+	 && (TYPE_MAXIMUM (pid_t) < INTMAX_MAX || errno != ERANGE)))
+    return -1;
+  pid = n;
 
-  colon = strchr (dot + 1, ':');
   /* After the `:', if there is one, comes the boot time.  */
-  n = 0;
-  if (colon)
+  switch (*colon)
     {
+    case 0:
+      n = 0;
+      break;
+
+    case ':':
+      if (! c_isdigit (colon[1]))
+	return -1;
       errno = 0;
-      n = strtoimax (colon + 1, NULL, 10);
+      n = strtoimax (colon + 1, &lfinfo_end, 10);
+      if (! (lfinfo_end == owner->user + lfinfolen
+	     && n <= TYPE_MAXIMUM (time_t)
+	     && (TYPE_MAXIMUM (time_t) < INTMAX_MAX || errno != ERANGE)))
+	return -1;
+      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;
+  boot_time = n;
 
   /* 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 (pid > 0
+               && (kill (pid, 0) >= 0 || errno == EPERM)
+	       && (boot_time == 0
+		   || 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
          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 +580,10 @@
       ret = 1;
     }
 
-  /* Avoid garbage.  */
-  if (owner == &local_owner || ret <= 0)
-    {
-      FREE_LOCK_INFO (*owner);
-    }
+  owner->at = at;
+  owner->dot = dot;
+  owner->pid = pid;
+  owner->boot_time = boot_time;
   return ret;
 }
 
@@ -551,29 +595,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 +685,12 @@
   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);
+      esprintf (lock_info.dot, " (pid %"pMd")", pid);
 
-      attack = call2 (intern ("ask-user-about-lock"), fn, build_string (locker));
+      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 +795,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;