Re: [bug #25294] assertion failure on dangling symlink to //

[Jim Meyering]

Eric Blake <address@hidden> 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 <address@hidden>
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