[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Jim Meyering
Subject: Re: [bug #25294] assertion failure on dangling symlink to //
Date: Sun, 11 Jan 2009 23:06:58 +0100

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
    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:

    ((Errno) == ENOENT || (Errno) == ENOSHARE)

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 

(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

+/* Applying Cygwin's stat to certain symlinks can evoke failure with a
+   nonstandard errno value of ENOSHARE.  Work around it. */
+#ifdef ENOSHARE
+    ((Errno) == ENOENT || (Errno) == ENOSHARE)
 /* 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);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]