Eric Blake <e...@byu.net> wrote:
> According to Eric Blake on 1/10/2009 6:58 PM:
>> Follow-up Comment #1, bug #25294 (project findutils):
>>
>> More info:
>>
>> The bug first surfaced in commit acb82fe, Nov 30 2008, at the point when Jim
>> provided a patch to gnulib fts to do fewer stat's when d_type is reliable.
>
> Jim, you may want to take a look at this assertion in find, caused by your
> improvements to gnulib fts to avoid excess stat's on new enough cygwin.
>
>>
>> The problem starts when readdir recognizes that it is too expensive to decide
>> what the type of a broken symlink is (at least on cygwin), and returns
>> DT_UNKNOWN (defined as 0). Next, fts calls fts_stat, which fails because the
>> symlink is broken, but on cygwin, a symlink to "//nowhere" fails with the
>> nonstandard errno==ENOSHARE rather than the more typical ENOENT, so no lstat
>> is attempted to see if the file might be a dangling symlink.
>>
>> So the bug might be in fts.c, line 1540, for not recognizing all cases of bad
>> symlinks (even ignoring cygwin's nonstandard ENOSHARE, what happens for a
>> looping symlink that returns ELOOP?). Would a readlink() be better than
>> lstat() to determine that a symlink exists but can't be followed?
>
> Currently, the ELOOP case triggers fts_info of FTS_NS rather than
> FTS_SLNONE. On seeing FTS_NS, find's consider_visiting calls
> symlink_loop, which performs a stat, but does not adjust state.have_stat
> to record this fact, leading to potentially duplicated stat's.
I tried your "find -L dir-containing-loop" example
on ext3, tmpfs, and xfs, and it appears d_type is always DT_LNK,
since the command didn't trigger a failed assertion:
$ mkdir e && ln -s loop e/loop && find -L e
e
find: `e/loop': Too many levels of symbolic links
[Exit 1]
so what do you think about changing the test from
if (errno == ENOENT
to e.g.,
if (FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK (errno)
Where the macro is defined like this:
#ifdef ENOSHARE
# define FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK(Errno) \
((Errno) == ENOENT || (Errno) == ENOSHARE)
#else
# define FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK(Errno) ((Errno) == ENOENT)
#endif
I checked a few Linux/GNU systems and found no ENOSHARE definition.
I.e,. how about this patch?
>From e56aaaa61c09cb915e2a3f24f9824be23ecd03fc Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
Date: Sun, 11 Jan 2009 22:57:37 +0100
Subject: [PATCH] fts: avoid assertion failure on Cygwin with find -L vs symlink
loop
* lib/fts.c (FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK): Define.
(fts_stat): Use it.
Report and diagnosis by Eric Blake.
Details in http://savannah.gnu.org/bugs/?25294.
---
lib/fts.c | 13 +++++++++++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/lib/fts.c b/lib/fts.c
index 836c179..c3dc855 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1,6 +1,6 @@
/* Traverse a file hierarchy.
- Copyright (C) 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
+ Copyright (C) 2004-2009 Free Software Foundation, Inc.
This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -130,6 +130,15 @@ enum
# define D_INO(dp) NOT_AN_INODE_NUMBER
#endif
+/* Applying Cygwin's stat to certain symlinks can evoke failure with a
+ nonstandard errno value of ENOSHARE. Work around it. */
+#ifdef ENOSHARE
+# define FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK(Errno) \
+ ((Errno) == ENOENT || (Errno) == ENOSHARE)
+#else
+# define FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK(Errno) ((Errno) == ENOENT)
+#endif
+
/* If there are more than this many entries in a directory,
and the conditions mentioned below are satisfied, then sort
the entries on inode number before any further processing. */
@@ -1544,7 +1553,7 @@ fts_stat(FTS *sp, register FTSENT *p, bool follow)
if (ISSET(FTS_LOGICAL) || follow) {
if (stat(p->fts_accpath, sbp)) {
saved_errno = errno;
- if (errno == ENOENT
+ if (FAILED_STAT_ERRNO_MAY_INDICATE_SYMLINK (errno)
&& lstat(p->fts_accpath, sbp) == 0) {
__set_errno (0);
return (FTS_SLNONE);
--
1.6.1.155.g1b01da
