[patch] nftw(): clarify dangling symlinks case

[patch] nftw(): clarify dangling symlinks case

[DJ Delorie]

In the event that nftw() encounters a symbolic link which refers to a
nonexisting file, the "struct stat" info will not have been filled in
if FTW_PHYS is not specified (because stat() is used instead of
lstat()).  This patch clarifies the documentation and corrects the
sample code to reflect this.

Reported to Fedora in https://bugzilla.redhat.com/show_bug.cgi?id=1422736

patch is against today's git

diff --git a/man3/ftw.3 b/man3/ftw.3
index b421c00..7b54621 100644
--- a/man3/ftw.3
+++ b/man3/ftw.3
@@ -165,7 +165,9 @@ is a symbolic link, and \fBFTW_PHYS\fP was set in \fIflags\fP.
 .B FTW_SLN
 .I fpath
 is a symbolic link pointing to a nonexistent file.
-(This occurs only if \fBFTW_PHYS\fP is not set.)
+(This occurs only if \fBFTW_PHYS\fP is not set, and the data
+.I sb
+points to during this callback will not be defined.)
 .PP
 The fourth argument that
 .BR nftw ()
@@ -421,13 +423,17 @@ static int
 display_info(const char *fpath, const struct stat *sb,
              int tflag, struct FTW *ftwbuf)
 {
-    printf("%\-3s %2d %7jd   %\-40s %d %s\\n",
-        (tflag == FTW_D) ?   "d"   : (tflag == FTW_DNR) ? "dnr" :
-        (tflag == FTW_DP) ?  "dp"  : (tflag == FTW_F) ?   "f" :
-        (tflag == FTW_NS) ?  "ns"  : (tflag == FTW_SL) ?  "sl" :
-        (tflag == FTW_SLN) ? "sln" : "???",
-        ftwbuf\->level, (intmax_t) sb\->st_size,
-        fpath, ftwbuf\->base, fpath + ftwbuf\->base);
+    if (tflag == FTW_SLN)
+        printf("sln %2d \-------   %-40s %d %s\n",
+            ftwbuf\->level, fpath, ftwbuf\->base, fpath + ftwbuf\->base);
+    else
+        printf("%\-3s %2d %7jd   %\-40s %d %s\\n",
+            (tflag == FTW_D) ?   "d"   : (tflag == FTW_DNR) ? "dnr" :
+            (tflag == FTW_DP) ?  "dp"  : (tflag == FTW_F) ?   "f" :
+            (tflag == FTW_NS) ?  "ns"  : (tflag == FTW_SL) ?  "sl" :
+            "???",
+            ftwbuf\->level, (intmax_t) sb\->st_size,
+            fpath, ftwbuf\->base, fpath + ftwbuf\->base);
     return 0;           /* To tell nftw() to continue */
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] nftw(): clarify dangling symlinks case

[Carlos O'Donell]

On 02/21/2017 08:06 PM, DJ Delorie wrote:
> In the event that nftw() encounters a symbolic link which refers to a
> nonexisting file, the "struct stat" info will not have been filled in
> if FTW_PHYS is not specified (because stat() is used instead of
> lstat()).  This patch clarifies the documentation and corrects the
> sample code to reflect this.
> 
> Reported to Fedora in https://bugzilla.redhat.com/show_bug.cgi?id=1422736
> 
> patch is against today's git

Looks good to me. Thanks for fixing the man page.

-- 
Cheers,
Carlos.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] nftw(): clarify dangling symlinks case

[Michael Kerrisk (man-pages)]

Hi DJ, Carlos,

Below is a copy of my response to the Red Hat bug report at
https://bugzilla.redhat.com/show_bug.cgi?id=1422736

On 22 February 2017 at 18:11, Carlos O'Donell <carlos-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 02/21/2017 08:06 PM, DJ Delorie wrote:
>> In the event that nftw() encounters a symbolic link which refers to a
>> nonexisting file, the "struct stat" info will not have been filled in
>> if FTW_PHYS is not specified (because stat() is used instead of
>> lstat()).  This patch clarifies the documentation and corrects the
>> sample code to reflect this.
>>
>> Reported to Fedora in https://bugzilla.redhat.com/show_bug.cgi?id=1422736
>>
>> patch is against today's git
>
> Looks good to me. Thanks for fixing the man page.
>
> --
> Cheers,
> Carlos.

(In reply to Carlos O'Donell from comment #7)
> (In reply to han pingtian from comment #5)
> > (In reply to DJ Delorie from comment #4)
> > > I've reproduced what you see.
> > >
> > > While it's not explicit in the nftw() man page, if you don't call nftw with
> > > FTW_PHYS, it only calls stat() on dangling symlinks, which of course fails.
> > > It returns FTW_SLN for those cases, and the stat data passed is not
> > > initialized.
> > >
> > If it knew it's an dangling symlink, why not returns the stat data of the
> > symblink itself. After all, it has known it's a dangling symblink.

((Yes.)

> Without FTW_PHYS the symlink _must_ be followed,

Why? (See below.)

> and if the symlink target
> doesn't exist then there is no stat-related data to be returned. If you want
> a mixed hybrid API somewhere between FTW_PHYS and not-FTW_PHYS then this
> needs to be discussed usptream. I think it's easy enough to just lstat()
> yourself on the broken symlink if that's what you need. Requiring the
> implementation to do the extra stat() will slow down performance for
> everyone that doesn't need and didn't ask for that information.
>
> In summary, having DJ adjust the documentation is the best way forward.

When I received the man-pages patch, I very nearly applied it, but
something niggled... (I have not applied the man-pages patch.)

I believe this is actually a (very longstanding) glibc bug. Here is
what POSIX says for nftw():

           FTW_NS    The  stat() function failed on the object because of
                     lack of  appropriate  permission.  The  stat  buffer
                     passed to fn is undefined. Failure of stat() for any
                     other reason is considered an error and nftw() shall
                     return −1.

           ....

           FTW_SLN   The  object is a symbolic link that does not name an
                     existing file.  (This condition shall only occur  if
                     the FTW_PHYS flag is not included in flags.)

Note that POSIX explicitly says that the stat buffer is undefined for
FTW_NS, but makes no such statement for FTW_SLN, with the implication
that the stat buffer is valid in this case.

This implies that FTW_SLN should work as Han Pingtian suggested: for a
dangling symlink, the lstat() information on the link should be
returned. This is certainly how I always understood things should
work. (But, obviously, I never tested this on glibc.)

So, what do other implementations do? Every other implementation that
I looked at, does return the lstat() information for the dangling
symlink. I looked at Solaris, OpenBSD, FreeBSD, and musl.  All of this
strongly suggests that glibc got it wrong.

For the points below, I used the following test program (and yes, I
realize by now that the FTW_NS treatment in this code is not correct;
I've fixed the man page already).

8x---8x---8x---8x---8x---8x---8x---8x---8x---8x---8x---8x---
/*#* t_nftw.c

   Copyright Michael Kerrisk 2000

   Demonstrate the use of the nftw(3) function.
*/
#define _GNU_SOURCE
#define _XOPEN_SOURCE 500
#include <ftw.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


static int
displayFileInfo(const char *fpath, const struct stat *sb,
            int tflag, struct FTW *ftwbuf)
{
    printf("%-3s %2d %7lld   %-30s %d %s (st_ino: %ld)\n",
        (tflag == FTW_D) ?   "d"   : (tflag == FTW_DNR) ? "dnr" :
    (tflag == FTW_DP) ?  "dp"  : (tflag == FTW_F) ?   "f" :
    (tflag == FTW_NS) ?  "ns"  : (tflag == FTW_SL) ?  "sl" :
    (tflag == FTW_SLN) ? "sln" : "???",
    ftwbuf->level, (long long) sb->st_size,
    fpath, ftwbuf->base, fpath + ftwbuf->base, (long) sb->st_ino);
    memset((void *) sb, 0, sizeof(struct stat));
    return 0;          /* To tell nftw() to continue */
}


int
main(int argc, char *argv[])
{
    int flags = 0;

    if (argc > 2 && strchr(argv[2], 'd') != NULL)
    flags |= FTW_DEPTH;
    if (argc > 2 && strchr(argv[2], 'p') != NULL)
    flags |= FTW_PHYS;

    if (nftw((argc < 2) ? "." : argv[1], displayFileInfo,
        20, flags) == -1) {
    perror("nftw");
    exit(EXIT_FAILURE);
    }

    exit(EXIT_SUCCESS);
}
8x---8x---8x---8x---8x---8x---8x---8x---8x---8x---8x---8x---


Solaris (Illumos source code)
usr/src/lib/libc/port/gen/nftw.c:

The following code causes the stat buffer to be populated with lstat()
infor in the FTW_SLN case:

        } else {
                /*
                 * Statf has failed. If stat was used instead of lstat,
                 * try using lstat. If lstat doesn't fail, "comp"
                 * must be a symbolic link pointing to a non-existent
                 * file. Such a symbolic link should be ignored.
                 * Also check the file type, if possible, for symbolic
                 * link.
                 */
                if ((vp->statf == stat) && (lstat(comp, &statb) >= 0) &&
                    ((statb.st_mode & S_IFMT) == S_IFLNK)) {

                        /*
                         * Ignore bad symbolic link, let "fn"
                         * report it.
                         */

                        errno = ENOENT;
                        type = FTW_SLN;
                } else {
                        type = FTW_NS;
        fail:


Testing shows that the link info *is* returned in the stat structure:

$ ls -li t
total 4
     45068 -rw-r--r--   1 mtk     csw      29 Feb 24 04:28 f
     45067 lrwxrwxrwx   1 mtk     csw       6 Feb 24 04:28 my_sln -> ssssss
     45069 lrwxrwxrwx   1 mtk     csw       1 Feb 24 04:28 sl_f -> f

$ ./a.out t
d    0       5   t                              0 t (st_ino: 45066)
sln  1       6   t/my_sln                       2 my_sln (st_ino: 45067)
f    1      29   t/f                            2 f (st_ino: 45068)
f    1      29   t/sl_f                         2 sl_f (st_ino: 45068)

======

OpenBSD

I didn't look at the source code, but the test gives the same results
as Solaris:

-bash-4.3$ ls -li
total 4
4693795 -rw-r--r--  1 mtk  mtk  29 Feb 24 04:37 f
4693796 lrwxr-xr-x  1 mtk  mtk   1 Feb 24 04:37 sl_f -> f
4693797 lrwxr-xr-x  1 mtk  mtk  11 Feb 24 04:37 sln -> jajdhfdskjh
-bash-4.3$ ./a.out t
d    0     512   t                              0 t (st_ino: 4693794)
f    1      29   t/f                            2 f (st_ino: 4693795)
f    1      29   t/sl_f                         2 sl_f (st_ino: 4693795)
sln  1      11   t/sln                          2 sln (st_ino: 4693797)

=====

FreeBSD

I don't have access to a FreeBSD test system at the moment, nut my
reading of the source code is that id delivers the same results as
Solaris and OpenBSD

See lib/libc/gen/nftw.c, where FTS_SLN is implemented using the
FTS_SLNONE option, and fts(3) on that system says:

              FTS_SLNONE  A symbolic link with a nonexistent target.  The
                          contents  of  the fts_statp field reference the
                          file characteristic information  for  the  sym‐
                          bolic link itself.

=====
musl libc

src/misc/nftw.c

        if ((flags & FTW_PHYS) ? lstat(path, &st) : stat(path, &st) < 0) {
                if (!(flags & FTW_PHYS) && errno==ENOENT && !lstat(path, &st))
                        type = FTW_SLN;
                else if (errno != EACCES) return -1;
                else type = FTW_NS;
        } else if (S_ISDIR(st.st_mode)) {
                if (access(path, R_OK) < 0) type = FTW_DNR;
                else if (flags & FTW_DEPTH) type = FTW_DP;
                else type = FTW_D;
        } else if (S_ISLNK(st.st_mode)) {
                if (flags & FTW_PHYS) type = FTW_SL;
                else type = FTW_SLN;
        } else {
                type = FTW_F;
        }


Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] nftw(): clarify dangling symlinks case

[Carlos O'Donell]

On 02/24/2017 03:58 AM, Michael Kerrisk (man-pages) wrote:
> So, what do other implementations do? Every other implementation that
> I looked at, does return the lstat() information for the dangling
> symlink. I looked at Solaris, OpenBSD, FreeBSD, and musl.  All of this
> strongly suggests that glibc got it wrong.

Michael,

Just because everyone else does it doesn't mean it's right, but it does
argue strongly for a portability case or just cargo-culting from Solaris.

However, what use case does that have? Why would you want the results 
of the lstat() if you specified !FTW_PHYS?

It seems to me that the lstat() is a waste of resources if the caller is
not interested in the results.

I think the bug is in the POSIX spec and they should have said that the
results of the stat buffer are undefined.

This should go to the Austin Group for clarification.

DJ,

Could you please ask the Austin Group[1] to clarify what is the result of
the stat buffer for dangling symlinks? Please post the bug once filed so
the rest of us can comment and provide our own guidance.

-- 
Cheers,
Carlos.

[1] http://austingroupbugs.net/main_page.php
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] nftw(): clarify dangling symlinks case

[Michael Kerrisk (man-pages)]

Hi Carlos,

On 24 February 2017 at 14:10, Carlos O'Donell <carlos-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On 02/24/2017 03:58 AM, Michael Kerrisk (man-pages) wrote:
>> So, what do other implementations do? Every other implementation that
>> I looked at, does return the lstat() information for the dangling
>> symlink. I looked at Solaris, OpenBSD, FreeBSD, and musl.  All of this
>> strongly suggests that glibc got it wrong.
>
> Michael,
>
> Just because everyone else does it doesn't mean it's right, but it does
> argue strongly for a portability case or just cargo-culting from Solaris.
>
> However, what use case does that have? Why would you want the results
> of the lstat() if you specified !FTW_PHYS?

Perhaps because it's the historical behavior that always existed and
therefore was standardized in POSIX? (The more I think about this, the
more it seems clear to met that POSIX clearly specifies the behavior
that every other implementations is providing.)

> It seems to me that the lstat() is a waste of resources if the caller is
> not interested in the results.

Why? You got to know that this is a dangling symlink and you got to
know information about the symlink. Seems pretty reasonable to me.
(You could equally say that stat()  is a waste of resources in many
other cases too, since the function called back by nftw() may not need
any of the info supplied in the stat buffer.)

> I think the bug is in the POSIX spec and they should have said that the
> results of the stat buffer are undefined.

I disagree. By and large POSIX standardizes existing implementation
behavior. The evidence suggests that that is what has happened in this
case also. (I do not know this for sure, of course. But 4/4
implementations is a fairly strong argument, I'd say.)

> This should go to the Austin Group for clarification.

I think POSIX is actualy rather clear already...

> Could you please ask the Austin Group[1] to clarify what is the result of
> the stat buffer for dangling symlinks? Please post the bug once filed so
> the rest of us can comment and provide our own guidance.

But why? The glibc implementation should ideally do what every other
implementation seems to do. Conversely: what's the argument for not
fixing it?

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html