Paul Eggert wrote:
> On 09/13/10 01:02, Jim Meyering wrote:
>> I ran this test: apply your patch, compile everything and
>> run coreutils' "make check" tests. With this change, 4 tests fail:
>>
>> $ grep FAIL tests/test-suite.log
>> FAIL: chgrp/posix-H (exit: 1)
>> FAIL: chgrp/recurse (exit: 1)
>> FAIL: du/deref (exit: 1)
>> FAIL: du/deref-args (exit: 1)
>
> Good thing I said that patch was "untested" :-). I just now tried the
> patch on my RHEL 5 host, and got the du failures but not the chgrp
> failures, I guess because the openattish support is somewhat limited
> in RHEL 5?
>
>> I haven't yet had time to look carefully, but those failures suggest that
>> such a change would also have to account for the FTS_COMFOLLOW option,
>> which may be set along with FTS_PHYSICAL.
>
> Yes, that could be it. I tried the following patch instead, and it
> fixed the du failures on RHEL 5. Maybe it will also fix the chgrp
> failures on your host. (I don't know offhand how to write a test case
> that will catch the race condition bug that this patch fixes.)
>
> diff --git a/lib/fts.c b/lib/fts.c
> index 4b89ee7..1daf69c 100644
> --- a/lib/fts.c
> +++ b/lib/fts.c
> @@ -290,10 +290,11 @@ fts_set_stat_required (FTSENT *p, bool required)
> /* FIXME: if others need this function, move it into lib/openat.c */
> static inline DIR *
> internal_function
> -opendirat (int fd, char const *dir)
> +opendirat (int fd, char const *dir, int extra_flags)
> {
> int new_fd = openat (fd, dir,
> - O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK);
> + (O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
> + | extra_flags));
> DIR *dirp;
>
> if (new_fd < 0)
> @@ -1237,7 +1238,11 @@ fts_build (register FTS *sp, int type)
> #else
> # define __opendir2(file, flag) \
> ( ! ISSET(FTS_NOCHDIR) && ISSET(FTS_CWDFD) \
> - ? opendirat(sp->fts_cwd_fd, file) \
> + ? opendirat(sp->fts_cwd_fd, file, \
> + ((ISSET(FTS_PHYSICAL) \
> + && ! (cur->fts_level == FTS_ROOTLEVEL \
> + && ISSET(FTS_COMFOLLOW))) \
> + ? O_NOFOLLOW : 0)) \
That passes all tests on Fedora 13 and looks sensible.
Thanks for the fix!
I agree that writing a test for that fix is probably more
trouble than it's worth.
