[PATCH v5 2/2] io: Use gnulib fts implementation (BZ 22944, BZ 20331)

[Collin Funk]

Florian Weimer <fweimer@redhat.com> writes:

> * Collin Funk:
>
>> This optimization is quite important. For example, 'chown' and 'chmod'
>> do not need to stat the files that they are processing. Using the
>> information from stat in those programs would likely introduce some
>> nasty TOCTOU races. Therefore, there is no need to do all this extra
>> work that glibc does:
>
> This makes sense.
>
> Still I think we need to version the symbol, with an internal flag that
> does more stat'ing with FTS_NOSTAT.  If we find problems pretty much
> immediately after updating glibc, it tends to suggest fairly widespread
> impact.

Here is a Gnulib patch which avoids the stat and preserves the old glibc
behavior:

diff --git a/lib/fts.c b/lib/fts.c
index c61e8d89d8..fb42f243a0 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1035,6 +1035,22 @@ check_for_dir:
                     if (! enter_dir (sp, p))
                       return NULL;
                   }
+
+                if (p->fts_info == FTS_NSOK)
+                  {
+                    if (S_ISREG (p->fts_statp->st_mode))
+                      p->fts_info = FTS_F;
+                    else if (S_ISDIR (p->fts_statp->st_mode))
+                      {
+                        if (! ISDOT (p->fts_name))
+                          p->fts_info = FTS_D;
+                        else
+                          {
+                            p->fts_info = (p->fts_level == FTS_ROOTLEVEL
+                                           ? FTS_D : FTS_DOT);
+                          }
+                      }
+                  }
                 return p;
         }
 cd_dot_dot:


Here is the test script with some typo fixes and the addition of
symbolic links, both dangling and existent:

    $ cat test.sh
    #!/bin/sh
    rm -rf testdir1 *.o main-glibc main-gnulib
    gnulib-tool --create-testdir --dir testdir1 fts
    (cd testdir1 && ./configure && make)
    mkdir -p a/b
    touch a/b/1 a/b/2 a/b/3
    ln -s $PWD/a/b/no-where $PWD/a/b/link-to-nowhere
    ln -s $PWD/a/b/1 $PWD/a/b/link
    gcc -c main.c -o main-glibc.o
    gcc -Itestdir1/gllib -Itestdir1 -DGNULIB=1 -c main.c -o main-gnulib.o
    gcc -o main-glibc main-glibc.o
    gcc -Ltestdir1/gllib -o main-gnulib main-gnulib.o -lgnu
    echo glibc
    ./main-glibc
    echo gnulib
    ./main-gnulib

And the output:

    $ ./test.sh
    [...]
    glibc
    a FTS_D
    a/b FTS_D
    a/b/1 FTS_F
    a/b/2 FTS_F
    a/b/3 FTS_F
    a/b/link-to-nowhere FTS_SLNONE
    a/b/link FTS_F
    a/b FTS_DP
    a FTS_DP
    gnulib
    a FTS_D
    a/b FTS_D
    a/b/1 FTS_F
    a/b/2 FTS_F
    a/b/3 FTS_F
    a/b/link-to-nowhere FTS_SLNONE
    a/b/link FTS_F
    a/b FTS_DP
    a FTS_DP

The FTS_NSOK is used for control flow, so setting ent->fts_info earlier
would be risky and likely introduce performance regressions. However,
ent->fts_statp->fts_info will always be initialized to zero or a file
type from dirp->d_type. Therefore, we can use the type just before
returning from fts_read.

Collin