Re: [PATCH 1/8] maint: fts.c: remove #if-0'd FTS_WHITEOUT code

[Jim Meyering]

Jim Meyering wrote:

> Eric Blake wrote:
>> On 08/18/2011 07:53 AM, Jim Meyering wrote:
>>> From: Jim Meyering<address@hidden>
>>>
>>> ---
>>>   lib/fts.c |   21 +--------------------
>>>   1 files changed, 1 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/lib/fts.c b/lib/fts.c
>>> index 7210c1b..c96dd9d 100644
>>> --- a/lib/fts.c
>>> +++ b/lib/fts.c
>>> @@ -1233,12 +1233,6 @@ fts_build (register FTS *sp, int type)
>>>            * Open the directory for reading.  If this fails, we're done.
>>>            * If being called from fts_read, set the fts_info field.
>>>            */
>>> -#if defined FTS_WHITEOUT&&  0
>>> -        if (ISSET(FTS_WHITEOUT))
>>> -                oflag = DTF_NODUP|DTF_REWIND;
>>> -        else
>>> -                oflag = DTF_HIDEW|DTF_NODUP|DTF_REWIND;
>
> Thanks for the feedback.
>
>> __opendir2 is really a useful entry point on BSD systems, which are
>> the only ones where whiteout files are supported.  Is our whiteout
>> support so bit-rotted that it really is time to remove it?
>
> You've just highlighted why that code is not worth preserving:
> it's BSD-specific.
>
> That code has been #if-0'd for years and no one has complained,
> and no gnulib client I know of uses FTS_WHITEOUT (they would
> have complained), so I think it's fine to remove it.
>
> The only reason I didn't remove it earlier was that I held out
> hope of syncing with some other version of fts.c.
>
>>> -#else
>>>   # define __opendir2(file, flag) \
>>>           opendirat((! ISSET(FTS_NOCHDIR)&&  ISSET(FTS_CWDFD)     \
>>>                      ? sp->fts_cwd_fd : AT_FDCWD),                \
>>> @@ -1249,7 +1243,7 @@ fts_build (register FTS *sp, int type)
>>>                       ? O_NOFOLLOW : 0)                           \
>>>                      | (ISSET (FTS_NOATIME) ? O_NOATIME : 0)),    \
>>>                     &dir_fd)
>>> -#endif
>>
>> Once you make this definition of __opendir2 unconditional, even for
>> BSD, I'd rather rename it to something more appropriate (lose the
>> leading __, and maybe even go with a more descriptive name like
>> fts_opendir), so that we aren't risking a conflict with the BSD
>> function.
>
> Thanks.  I nearly did that, but lazily kept the change minimal.
> I'll merge in the renaming to fts_opendir.

Rebasing would have been a pain, but editing the output of this

    git format-pat --stdout master > diff

and reapplying that to a pristine branch off of master worked fine.
Comparing the branches with:

  git diff 2011-228-fts-surgery 2011-231-fts-surgery

gives this:

diff --git a/lib/fts.c b/lib/fts.c
index 26cd444..736350e 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1231,7 +1231,7 @@ set_stat_type (struct stat *st, unsigned int dtype)
     }                                           \
   while (0)

-#define __opendir2(file, Pdir_fd)                               \
+#define fts_opendir(file, Pdir_fd)                              \
         opendirat((! ISSET(FTS_NOCHDIR) && ISSET(FTS_CWDFD)     \
                    ? sp->fts_cwd_fd : AT_FDCWD),                \
                   file,                                         \
@@ -1298,7 +1298,7 @@ fts_build (register FTS *sp, int type)
           {
             /* Open the directory for reading.  If this fails, we're done.
                If being called from fts_read, set the fts_info field. */
-            if ((cur->fts_dirp = __opendir2(cur->fts_accpath, &dir_fd)) == 
NULL)
+            if ((cur->fts_dirp = fts_opendir(cur->fts_accpath, &dir_fd)) == 
NULL)
               {
                 if (type == BREAD)
                   {

I updated the log to say this:

    maint: fts: give __opendir2 a new parameter and rename

    * lib/fts.c (__opendir2): Give it a new parameter, Pdir_fd, rather
    than surreptitiously using sole caller's "dir_fd".
    (fts_opendir): Rename from __opendir2.