* bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.
@ 2013-02-24 22:48 Paul Eggert
2013-02-25 19:57 ` Glenn Morris
2013-02-26 22:19 ` bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes Paul Eggert
0 siblings, 2 replies; 14+ messages in thread
From: Paul Eggert @ 2013-02-24 22:48 UTC (permalink / raw)
To: 13807
[-- Attachment #1: Type: text/plain, Size: 152 bytes --]
Tags: patch
Attached is a proposed cleanup patch, following up on the thread in
<http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.
[-- Attachment #2: filelock.txt --]
[-- Type: text/plain, Size: 5507 bytes --]
=== modified file 'etc/ChangeLog'
--- etc/ChangeLog 2013-02-19 14:44:03 +0000
+++ etc/ChangeLog 2013-02-24 22:44:04 +0000
@@ -1,3 +1,7 @@
+2013-02-24 Paul Eggert <eggert@cs.ucla.edu>
+
+ * NEWS: The lock for 'DIR/FILE' is now always 'DIR/.#FILE'.
+
2013-02-14 Michael Albinus <michael.albinus@gmx.de>
* NEWS: Tramp methods "scpc" and "rsyncc" are discontinued.
=== modified file 'etc/NEWS'
--- etc/NEWS 2013-02-21 06:55:19 +0000
+++ etc/NEWS 2013-02-24 22:44:04 +0000
@@ -316,6 +316,19 @@
** 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 always '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. This approach had race conditions that could
+not be easily fixed. File names beginning with '.#' are unusual
+enough that the fallbacks were more trouble than they were worth, so
+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).
+
** 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-24 19:45:17 +0000
+++ src/ChangeLog 2013-02-24 22:44:04 +0000
@@ -1,3 +1,16 @@
+2013-02-24 Paul Eggert <eggert@cs.ucla.edu>
+
+ The lock for 'DIR/FILE' is now always 'DIR/.#FILE'.
+ * filelock.c (MAKE_LOCK_NAME, fill_in_lock_file_name):
+ Don't create DIR/.#FILE.0 through DIR/.#FILE.9, as this was
+ more trouble than it was worth and led to race conditions of its own.
+ (MAKE_LOCK_NAME, unlock_file, Ffile_locked_p):
+ Use SAFE_ALLOCA to avoid problems with long file names.
+ (lock_file_1): Don't inspect errno if symlink call succeeds;
+ that's not portable.
+ (lock_file): Document that this functionn can return if
+ lock creation fails.
+
2013-02-24 Andreas Schwab <schwab@linux-m68k.org>
* xdisp.c (set_message): Only check for debug-on-message if STRING
=== modified file 'src/filelock.c'
--- src/filelock.c 2013-02-24 19:45:17 +0000
+++ src/filelock.c 2013-02-24 22:44:04 +0000
@@ -289,44 +289,22 @@
/* Write the name of the lock file for FN into LFNAME. 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. */
+ that of FN plus two more for the leading `.#' plus one for the null. */
#define MAKE_LOCK_NAME(lock, file) \
- (lock = alloca (SBYTES (file) + 2 + 1 + 1 + 1), \
- fill_in_lock_file_name (lock, (file)))
+ (lock = SAFE_ALLOCA (SBYTES (file) + 2 + 1), \
+ fill_in_lock_file_name (lock, file))
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 && !S_ISLNK (st.st_mode))
- {
- 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] = '#';
+ strcpy (lockfile + dirlen + 2, base);
}
/* Lock the lock file named LFNAME.
@@ -357,7 +335,7 @@
user_name, host_name, pid, boot);
err = symlink (lock_info_str, lfname);
- if (errno == EEXIST && force)
+ if (err != 0 && errno == EEXIST && force)
{
unlink (lfname);
err = symlink (lock_info_str, lfname);
@@ -520,6 +498,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
@@ -599,9 +578,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);
@@ -610,6 +590,8 @@
if (current_lock_owner (0, lfname) == 2)
unlink (lfname);
+
+ SAFE_FREE ();
}
void
@@ -675,9 +657,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);
@@ -694,6 +677,7 @@
if (owner > 0)
FREE_LOCK_INFO (locker);
+ SAFE_FREE ();
return ret;
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.
2013-02-24 22:48 bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE' Paul Eggert
@ 2013-02-25 19:57 ` Glenn Morris
2013-02-25 23:40 ` Paul Eggert
2013-02-26 22:19 ` bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes Paul Eggert
1 sibling, 1 reply; 14+ messages in thread
From: Glenn Morris @ 2013-02-25 19:57 UTC (permalink / raw)
To: Paul Eggert; +Cc: 13807
Paul Eggert wrote:
> Attached is a proposed cleanup patch, following up on the thread in
> <http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.
AFAICS, all that thread says is "we should remove this because it never
worked", when in fact it did work just fine until recently.
So the motivation for this seems to be entirely as given in the NEWS
(which is not really where it belongs IMO) for this change. Where it
just says "this was more trouble than it was worth and led to race
conditions of its own". No-one ever reported any (non-theoretical)
problems with it in practice, AFAIK.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE'.
2013-02-25 19:57 ` Glenn Morris
@ 2013-02-25 23:40 ` Paul Eggert
0 siblings, 0 replies; 14+ messages in thread
From: Paul Eggert @ 2013-02-25 23:40 UTC (permalink / raw)
To: Glenn Morris; +Cc: 13807
[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]
On 02/25/13 11:57, Glenn Morris wrote:
>> <http://lists.gnu.org/archive/html/emacs-devel/2013-02/msg00531.html>.
>
> AFAICS, all that thread says is "we should remove this because it never
> worked", when in fact it did work just fine until recently.
Yes, the discussion should have been clearer. This patch was prompted
by a code inspection after fixing the bug mentioned in that thread; the patch
does not fix the bug (the bug's already fixed). I tried to clarify
this in the revised patch (attached).
> So the motivation for this seems to be entirely as given in the NEWS
> (which is not really where it belongs IMO)
OK, I moved the motivation out of NEWS and into the ChangeLog entry.
> No-one ever reported any (non-theoretical)
> problems with it in practice, AFAIK.
The problem is more likely to happen with today's changes to the
MS-Windows side. And even if the problem was less likely, it's still
a race condition that should get fixed -- the point of that lock
file is to avoid races, after all.
[-- Attachment #2: filelock.txt --]
[-- Type: text/plain, Size: 6410 bytes --]
=== modified file 'etc/ChangeLog'
--- etc/ChangeLog 2013-02-25 17:01:41 +0000
+++ etc/ChangeLog 2013-02-25 19:42:26 +0000
@@ -1,5 +1,7 @@
2013-02-25 Paul Eggert <eggert@cs.ucla.edu>
+ * NEWS: The lock for 'DIR/FILE' is now always 'DIR/.#FILE' (Bug#13807).
+
Simplify data_start configuration (Bug#13783).
* NEWS: Document removal of --with-crt-dir.
* PROBLEMS (LIBS_SYSTEM, LIBS_MACHINE, LIBS_STANDARD): Remove.
=== modified file 'etc/NEWS'
--- etc/NEWS 2013-02-25 17:36:03 +0000
+++ etc/NEWS 2013-02-25 23:00:06 +0000
@@ -319,6 +319,14 @@
** 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 always '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.
+
** 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-25 17:36:03 +0000
+++ src/ChangeLog 2013-02-25 23:33:21 +0000
@@ -1,3 +1,26 @@
+2013-02-25 Paul Eggert <eggert@cs.ucla.edu>
+
+ The lock for 'DIR/FILE' is now always 'DIR/.#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>.
+ * filelock.c (MAKE_LOCK_NAME, fill_in_lock_file_name):
+ Don't create DIR/.#FILE.0 through DIR/.#FILE.9.
+ (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-25 Eli Zaretskii <eliz@gnu.org>
Implement CLASH_DETECTION for MS-Windows.
=== modified file 'src/filelock.c'
--- src/filelock.c 2013-02-25 17:36:03 +0000
+++ src/filelock.c 2013-02-25 23:35:55 +0000
@@ -292,53 +292,22 @@
/* 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
+ will be that of FNAME plus two more for the leading `.#' 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
+#define MAKE_LOCK_NAME(lockname, fname) \
+ (lockname = SAFE_ALLOCA (SBYTES (fname) + 2 + 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] = '#';
+ strcpy (lockfile + dirlen + 2, base);
}
static int
@@ -374,7 +343,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 +403,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 +566,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 +652,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 +664,8 @@
if (current_lock_owner (0, lfname) == 2)
unlink (lfname);
+
+ SAFE_FREE ();
}
void
@@ -756,9 +731,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 +751,7 @@
if (owner > 0)
FREE_LOCK_INFO (locker);
+ SAFE_FREE ();
return ret;
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-02-24 22:48 bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE' Paul Eggert
2013-02-25 19:57 ` Glenn Morris
@ 2013-02-26 22:19 ` Paul Eggert
2013-02-27 18:49 ` Eli Zaretskii
1 sibling, 1 reply; 14+ messages in thread
From: Paul Eggert @ 2013-02-26 22:19 UTC (permalink / raw)
To: 13807
[-- 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;
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-02-26 22:19 ` bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes Paul Eggert
@ 2013-02-27 18:49 ` Eli Zaretskii
2013-03-02 20:43 ` Paul Eggert
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2013-02-27 18:49 UTC (permalink / raw)
To: Paul Eggert; +Cc: 13807
> Date: Tue, 26 Feb 2013 14:19:52 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
>
> 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.
I think I found a better solution (committed as trunk revision
111888). Instead of using 'emacs_open', which just calls 'open', I
now use '_sopen', which allows to specify sharing restrictions for
accessing the same file via other file descriptors, both by the same
Emacs which is creating the lock file, and by other processes. The
sharing flag I used denies any kind of access to the same file until
the file descriptor returned by _sopen is closed. This makes the
whole open/write/close sequence in create_lock_file atomic, as far as
other potential readers or writers of the file are concerned -- they
will not be able to access the file until its contents is ready.
I tested this with one NFS-mounted volume that can be accessed from
both Windows and a GNU/Linux box. The results were that when Emacs
running on GNU/Linux had the file locked, Emacs running on Windows
would refrain from locking it, and vice versa when the file was locked
by Emacs running on Windows and a GNU/Linux Emacs would try to lock
it. This is IMO better than ignoring the locks from the other
platform, because the latter means each Emacs thinks it has an
exclusive lock on the file, while in fact there are several lockers.
I can make a similar change in read_lock_data, but I don't think it's
needed there, since 'symlink' on Posix hosts is atomic, and thus Emacs
on Windows will always either see the symlink in its final state or
not at all.
So I think with this change, there's no longer a need to use different
file names for the lock files on Windows and on Posix hosts.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-02-27 18:49 ` Eli Zaretskii
@ 2013-03-02 20:43 ` Paul Eggert
2013-03-02 21:17 ` Eli Zaretskii
0 siblings, 1 reply; 14+ messages in thread
From: Paul Eggert @ 2013-03-02 20:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 13807
On 02/27/2013 10:49 AM, Eli Zaretskii wrote:
> The results were that when Emacs running on GNU/Linux had the file
> locked, Emacs running on Windows would refrain from locking it, and
> vice versa when the file was locked by Emacs running on Windows and
> a GNU/Linux Emacs would try to lock it.
Unfortunately when (for example) the GNU/Linux Emacs refrained from
locking the file, what it was actually doing was using its own
separate lock file, whose name it got in a buggy way. That is, when
locking FILE it discovered a regular file .#FILE (the MS-Windows lock
file) and then decided to use a symlink .#FILE.0, thus ignoring the
MS-Windows lock.
The process of guessing a lock file name by appending ".0" is
obviously flaky, as it's prone to races. The recent MS-Windows
changes have made the races more likely, but they were present even
before the changes. Emacs should not guess the lock file name.
For now, I've installed the patch as trunk bzr 111918, as it fixes these
races. This patch causes Emacs to use a different lock file name
.#-FILE for MS-Windows than the usual lock file .#FILE for GNU/Linux,
which is not good, but Emacs was using different lock files anyway,
and a virtue of the patch is that any problems in the MS/Windows
implementation won't get in the way of GNU/Linux users on a networked
file system.
I will look into adjusting Emacs so that it uses the same lock file
name .#FILE for both GNU/Linux and MS-Windows, which would be nicer
than the patched Emacs. This will take some more thought, though.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-02 20:43 ` Paul Eggert
@ 2013-03-02 21:17 ` Eli Zaretskii
2013-03-02 22:37 ` Paul Eggert
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2013-03-02 21:17 UTC (permalink / raw)
To: Paul Eggert; +Cc: 13807
> Date: Sat, 02 Mar 2013 12:43:05 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 13807@debbugs.gnu.org
>
> On 02/27/2013 10:49 AM, Eli Zaretskii wrote:
>
> > The results were that when Emacs running on GNU/Linux had the file
> > locked, Emacs running on Windows would refrain from locking it, and
> > vice versa when the file was locked by Emacs running on Windows and
> > a GNU/Linux Emacs would try to lock it.
>
> Unfortunately when (for example) the GNU/Linux Emacs refrained from
> locking the file, what it was actually doing was using its own
> separate lock file, whose name it got in a buggy way. That is, when
> locking FILE it discovered a regular file .#FILE (the MS-Windows lock
> file) and then decided to use a symlink .#FILE.0, thus ignoring the
> MS-Windows lock.
That's not what happened in my testing. There, there was no lock file
named .#FILE.0 or anything like that. Can you describe your testing
in more details, and what versions of NFS and Windows did you use?
> The process of guessing a lock file name by appending ".0" is
> obviously flaky, as it's prone to races. The recent MS-Windows
> changes have made the races more likely, but they were present even
> before the changes. Emacs should not guess the lock file name.
That's a different issue, though.
> For now, I've installed the patch as trunk bzr 111918, as it fixes these
> races. This patch causes Emacs to use a different lock file name
> .#-FILE for MS-Windows than the usual lock file .#FILE for GNU/Linux,
> which is not good, but Emacs was using different lock files anyway,
> and a virtue of the patch is that any problems in the MS/Windows
> implementation won't get in the way of GNU/Linux users on a networked
> file system.
I wish you didn't install the .#-FILE part. It is no longer
justified.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-02 21:17 ` Eli Zaretskii
@ 2013-03-02 22:37 ` Paul Eggert
2013-03-03 16:39 ` Eli Zaretskii
0 siblings, 1 reply; 14+ messages in thread
From: Paul Eggert @ 2013-03-02 22:37 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 13807
[-- 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;
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-02 22:37 ` Paul Eggert
@ 2013-03-03 16:39 ` Eli Zaretskii
2013-03-03 23:56 ` Paul Eggert
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2013-03-03 16:39 UTC (permalink / raw)
To: Paul Eggert, Stefan Monnier; +Cc: 13807
> Date: Sat, 02 Mar 2013 14:37:29 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: 13807@debbugs.gnu.org
>
> 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.
But this is not a faithful reproduction of what happens when .#FILE is
created by Emacs running on Windows. Because of the way Emacs on
Windows creates/opens this file, it is effectively inaccessible to any
other process, even _after_ the file is written and its handle closed.
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.) And
that caused Emacs on GNU/Linux to refrain from locking the file.
In addition, 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? There are no more
.#FILE.0 alternative names.
So I still don't see the reason for using different names on Windows
and Posix hosts.
> 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.
But these issues are unrelated to the MS-Windows build of Emacs. They
existed since about forever, and we never cared. Why is it suddenly
so important that this feature works with 110% reliability, something
it never did? Am I the only one who thinks we are way past the point
of diminishing returns here1? Stefan? Anyone?
> 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.
I think we are wasting our time and energy here. Nothing terrible
will happen if locking silently fails from time to time, as it always
did. If we want, we could even optionally make these failures be
announced, by looking at the value lock_file returns, which we
currently ignore.
But if we do go in the direction you suggest, I think we will need to
provide two completely separate implementations for create_lock_file,
one for Posix, the other for MS-Windows. This is because the
primitives you used in your suggested patch have problems on Windows:
'rename' is not atomic when it needs to delete the target (this is not
supported by the MS 'rename', so we emulate this in w32.c:sys_rename),
and hard links are only supported on NTFS, so editing files on FAT32
volumes (flash drives are normally formatted this way) will be unable
to lock files. By contrast, using '_sopen', like I did in the current
code, really does make creation of the lock file atomic, and works
with any filesystem supported by Windows, including NFS-mounted
volumes.
So, unless there's a way to do on Posix platforms what '_sopen' does
on Windows (perhaps some file-locking feature? I don't know enough
about that), I think having 2 completely separate implementations will
be cleaner and more maintainable. (Assuming, that is, that we really
do want to make file locking so bulletproof.)
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-03 16:39 ` Eli Zaretskii
@ 2013-03-03 23:56 ` Paul Eggert
2013-03-04 16:50 ` Eli Zaretskii
0 siblings, 1 reply; 14+ messages in thread
From: Paul Eggert @ 2013-03-03 23:56 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 13807
[-- Attachment #1: Type: text/plain, Size: 2274 bytes --]
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.
[-- Attachment #2: filelock2.txt --]
[-- Type: text/plain, Size: 24264 bytes --]
=== 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 <eggert@cs.ucla.edu>
+
+ FILE's lock is now always .#FILE and may be a regular file (Bug#13807).
+ * NEWS: Document this.
+
2013-03-02 Bill Wohler <wohler@newt.com>
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 <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 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 <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.
=== 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 `user@host.pid'. This avoids a single
mount (== failure) point for lock files.
@@ -100,10 +102,21 @@
--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'. 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. */
\f
/* Return the time of the last system boot. */
@@ -284,30 +297,31 @@
}
#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;
- 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 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 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);
}
\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 +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 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;
+ 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 *);
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-03 23:56 ` Paul Eggert
@ 2013-03-04 16:50 ` Eli Zaretskii
2013-03-05 2:25 ` Paul Eggert
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2013-03-04 16:50 UTC (permalink / raw)
To: Paul Eggert; +Cc: 13807
> Date: Sun, 03 Mar 2013 15:56:03 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: Stefan Monnier <monnier@iro.umontreal.ca>, 13807@debbugs.gnu.org
>
> > 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.
Like it or not, it's out there, and others might bump into it.
> > 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.
I meant what you wrote here:
> 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.
These issues are unrelated to whether Emacs on Windows does or doesn't
lock files. They existed before, as did the issue with FAT32 volumes
being used from Posix hosts.
And I think you exaggerate the probability of having Emacs running on
Windows to access via NFS files shared with Posix systems. In my
experience, this is quite rare (as is having people use Emacs on
Windows in general).
> + 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;
This will need a no-op emulation of fchmod for Windows (since a file
created here will be world-writable anyway). Other than that, I don't
see any problems with your changes (but I didn't try to build with
them, I only read them).
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-04 16:50 ` Eli Zaretskii
@ 2013-03-05 2:25 ` Paul Eggert
2013-03-05 18:38 ` Eli Zaretskii
0 siblings, 1 reply; 14+ messages in thread
From: Paul Eggert @ 2013-03-05 2:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 13807
[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]
On 03/04/13 08:50, Eli Zaretskii wrote:
> it's out there, and others might bump into it.
OK. Luckily, with the latest proposed patch, Emacs users will be less
likely to bump into the NFS problem, since Emacs won't attempt to create
lock files that exploit the issue.
> These issues are unrelated to whether Emacs on Windows does or doesn't
> lock files. They existed before, as did the issue with FAT32 volumes
> being used from Posix hosts.
Yes, quite right. Sorry I misunderstood you.
> I think you exaggerate the probability of having Emacs running on
> Windows to access via NFS files shared with Posix systems.
Possibly. It depends on local practice. Many locations don't use NFS
at all. Around here we use NFS heavily.
> This will need a no-op emulation of fchmod for Windows (since a file
> created here will be world-writable anyway).
OK, thanks. Also, older POSIXish hosts that lack mkstemp won't need
the fchmod either. I added the following to try to address these two points.
Revised complete patch attached, relative to trunk bzr 111938.
=== modified file 'src/filelock.c'
--- src/filelock.c 2013-03-04 19:27:39 +0000
+++ src/filelock.c 2013-03-04 19:36:45 +0000
@@ -407,15 +407,21 @@
USE_SAFE_ALLOCA;
char *nonce = SAFE_ALLOCA (lfdirlen + sizeof nonce_base);
int fd;
+ bool need_fchmod;
+ mode_t world_readable = S_IRUSR | S_IRGRP | S_IROTH;
memcpy (nonce, lfname, lfdirlen);
strcpy (nonce + lfdirlen, nonce_base);
#if HAVE_MKSTEMP
+ /* Prefer mkstemp if available, as it avoids a race between
+ mktemp and emacs_open. */
fd = mkstemp (nonce);
+ need_fchmod = 1;
#else
mktemp (nonce);
fd = emacs_open (nonce, O_WRONLY | O_CREAT | O_EXCL | O_BINARY,
- S_IRUSR | S_IWUSR);
+ world_readable);
+ need_fchmod = 0;
#endif
if (fd < 0)
@@ -425,7 +431,7 @@
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)
+ || (need_fchmod && fchmod (fd, world_readable) != 0))
err = errno;
if (emacs_close (fd) != 0)
err = errno;
=== modified file 'src/w32.c'
--- src/w32.c 2013-03-03 23:12:54 +0000
+++ src/w32.c 2013-03-05 01:42:12 +0000
@@ -3416,6 +3416,12 @@
}
int
+fchmod (int fd, mode_t mode)
+{
+ return 0;
+}
+
+int
sys_rename_replace (const char *oldname, const char *newname, BOOL force)
{
BOOL result;
=== modified file 'src/w32.h'
--- src/w32.h 2013-03-03 23:12:54 +0000
+++ src/w32.h 2013-03-05 01:42:12 +0000
@@ -186,6 +186,7 @@
extern void srandom (int);
extern int random (void);
+extern int fchmod (int, mode_t);
extern int sys_rename_replace (char const *, char const *, BOOL);
extern int sys_pipe (int *);
[-- Attachment #2: filelock2.txt --]
[-- Type: text/plain, Size: 24447 bytes --]
=== modified file 'etc/ChangeLog'
--- etc/ChangeLog 2013-03-03 00:02:19 +0000
+++ etc/ChangeLog 2013-03-04 17:49:23 +0000
@@ -1,3 +1,8 @@
+2013-03-04 Paul Eggert <eggert@cs.ucla.edu>
+
+ FILE's lock is now always .#FILE and may be a regular file (Bug#13807).
+ * NEWS: Document this.
+
2013-03-02 Bill Wohler <wohler@newt.com>
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-04 17:35:29 +0000
+++ src/ChangeLog 2013-03-05 01:42:12 +0000
@@ -1,5 +1,56 @@
2013-03-04 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 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, fchmod, 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. 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.
+ (fchmod): New dummy function.
+ * w32.h (sys_rename_replace, fchmod): New decls.
+
+2013-03-04 Paul Eggert <eggert@cs.ucla.edu>
+
Fix misuse of ImageMagick that caused core dump (Bug#13846).
* image.c (imagemagick_load_image): Calculate height and width
after flattening the image, not before.
=== modified file 'src/filelock.c'
--- src/filelock.c 2013-03-02 21:05:52 +0000
+++ src/filelock.c 2013-03-04 19:36:45 +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,21 @@
--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'. 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. */
\f
/* Return the time of the last system boot. */
@@ -284,30 +297,31 @@
}
#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;
- 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 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 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,127 @@
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;
+ bool need_fchmod;
+ mode_t world_readable = S_IRUSR | S_IRGRP | S_IROTH;
+ memcpy (nonce, lfname, lfdirlen);
+ strcpy (nonce + lfdirlen, nonce_base);
+
+#if HAVE_MKSTEMP
+ /* Prefer mkstemp if available, as it avoids a race between
+ mktemp and emacs_open. */
+ fd = mkstemp (nonce);
+ need_fchmod = 1;
+#else
+ mktemp (nonce);
+ fd = emacs_open (nonce, O_WRONLY | O_CREAT | O_EXCL | O_BINARY,
+ world_readable);
+ need_fchmod = 0;
#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
+ || (need_fchmod && fchmod (fd, world_readable) != 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 +461,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 +481,39 @@
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)
+ {
+ ptrdiff_t read_bytes = emacs_read (fd, lfinfo, MAX_LFINFO + 1);
+ int read_errno = errno;
+ if (emacs_close (fd) != 0)
+ return -1;
+ errno = read_errno;
+ return read_bytes;
+ }
+
+ 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 +525,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 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;
+ 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 +604,6 @@
ret = 1;
}
- /* Avoid garbage. */
- if (owner == &local_owner || ret <= 0)
- {
- FREE_LOCK_INFO (*owner);
- }
return ret;
}
@@ -551,29 +615,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 +705,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 +819,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-05 01:42:12 +0000
@@ -3416,7 +3416,13 @@
}
int
-sys_rename (const char * oldname, const char * newname)
+fchmod (int fd, mode_t mode)
+{
+ return 0;
+}
+
+int
+sys_rename_replace (const char *oldname, const char *newname, BOOL force)
{
BOOL result;
char temp[MAX_PATH];
@@ -3472,7 +3478,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 +3496,7 @@
result = rename (temp, newname);
- if (result < 0)
+ if (result < 0 && force)
{
DWORD w32err = GetLastError ();
@@ -3530,6 +3536,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-05 01:42:12 +0000
@@ -186,6 +186,8 @@
extern void srandom (int);
extern int random (void);
+extern int fchmod (int, mode_t);
+extern int sys_rename_replace (char const *, char const *, BOOL);
extern int sys_pipe (int *);
extern void set_process_dir (char *);
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-05 2:25 ` Paul Eggert
@ 2013-03-05 18:38 ` Eli Zaretskii
2013-03-05 22:38 ` Paul Eggert
0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2013-03-05 18:38 UTC (permalink / raw)
To: Paul Eggert; +Cc: 13807
> Date: Mon, 04 Mar 2013 18:25:32 -0800
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: monnier@iro.umontreal.ca, 13807@debbugs.gnu.org
>
> > This will need a no-op emulation of fchmod for Windows (since a file
> > created here will be world-writable anyway).
>
> OK, thanks. Also, older POSIXish hosts that lack mkstemp won't need
> the fchmod either. I added the following to try to address these two points.
> Revised complete patch attached, relative to trunk bzr 111938.
Thanks, I have 2 more nits.
> + 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)
> + {
> + ptrdiff_t read_bytes = emacs_read (fd, lfinfo, MAX_LFINFO + 1);
> + int read_errno = errno;
> + if (emacs_close (fd) != 0)
> + return -1;
> + errno = read_errno;
> + return read_bytes;
> + }
> +
> + if (errno != ELOOP)
> + return -1;
We will need to define away O_NOFOLLOW and ELOOP, to get this to
compile on Windows. I think the right place for the former is
nt/inc/unistd.h, near O_NOCTTY, and for the latter nt/inc/ms-w32.h,
where ENOTSUP is defined.
Other than that, I think this is OK. Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes
2013-03-05 18:38 ` Eli Zaretskii
@ 2013-03-05 22:38 ` Paul Eggert
0 siblings, 0 replies; 14+ messages in thread
From: Paul Eggert @ 2013-03-05 22:38 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 13807-done
On 03/05/13 10:38, Eli Zaretskii wrote:
> We will need to define away O_NOFOLLOW and ELOOP, to get this to
> compile on Windows. I think the right place for the former is
> nt/inc/unistd.h, near O_NOCTTY, and for the latter nt/inc/ms-w32.h,
> where ENOTSUP is defined.
Thanks, I did the former, but for the latter it's possible ELOOP
won't be defined on a (very old) POSIXish host, so I put a conditional
definition for it into filelock.c itself, which should work for
MS-Windows as well. Installed as trunk bzr 111948 and marking
this as done.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-03-05 22:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-24 22:48 bug#13807: The lock for 'DIR/FILE' should always be 'DIR/.#FILE' Paul Eggert
2013-02-25 19:57 ` Glenn Morris
2013-02-25 23:40 ` Paul Eggert
2013-02-26 22:19 ` bug#13807: updated version to avoid MS-Windows vs non-MS-Windows clashes Paul Eggert
2013-02-27 18:49 ` Eli Zaretskii
2013-03-02 20:43 ` Paul Eggert
2013-03-02 21:17 ` Eli Zaretskii
2013-03-02 22:37 ` Paul Eggert
2013-03-03 16:39 ` Eli Zaretskii
2013-03-03 23:56 ` Paul Eggert
2013-03-04 16:50 ` Eli Zaretskii
2013-03-05 2:25 ` Paul Eggert
2013-03-05 18:38 ` Eli Zaretskii
2013-03-05 22:38 ` Paul Eggert
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).