[RFC] Call lstat() before setting flags to FTW_SLN

  POSIX.1-2008 doesn't describe this behavior explicitly in this case.
However, passing a stat buffer returned by a failed fstatat() doesn't
seem the correct behavior either.

Any comments?

---8<---

POSIX.1-2008 states the following for ftw():

    If the object is a symbolic link and stat() failed, it is unspecified
    whether ftw() passes FTW_SL or FTW_NS to the user-supplied function.

However, nftw() defines FTW_SLN, which is returned if "the object is a
symbolic link that does not name an existing file".

It also states:

    At each file it encounters, nftw() shall call the user-supplied function
    fn with four arguments:
    ...
    - The second argument is a pointer to the stat buffer containing
      information on the object, filled in as if fstatat(), stat(), or
      lstat() had been called to retrieve the information.

In the current implementation, when FTW_PHYS is not included and the object
is a symlink that doesn't name an existing file, the call to fstatat()
returns -1, flags is set to FTW_SLN and fn is called with a stat buffer
from a failed fstatat() call.

2017-02-24  Tulio Magno Quites Machado Filho  <tuliom@linux.vnet.ibm.com>

	* io/ftw.c (process_entry): Call LXSTAT() if FXSTATAT() failed
	and the object is a symbolic link.
---
 io/ftw.c | 2 --
 1 file changed, 2 deletions(-)
  

"Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> writes:
> POSIX.1-2008 doesn't describe this behavior explicitly in this case.

See also https://bugzilla.redhat.com/show_bug.cgi?id=1422736
and http://austingroupbugs.net/view.php?id=1121

The spec also says "If FTW_PHYS is clear ... nftw() shall follow links
instead of reporting them" but calling lstat() *is* reporting them,
which the spec specifically forbids, if you read it that way.

Hence, the clarification I requested from the Austin Group.  If the spec
says we're not allowed to call lstat(), your patch is inappropriate.  If
we are allowed to call lstat(), I think your patch is wrong - it may
return FTW_NS for cases where lstat() might fail, where FTW_SLN should
have been returned.  I can't think of any such cases, but I've been
surprised before.  Also, cases where FTW_NS should be returned due to
errors reading *non*dangling links should be preserved.

(I suspect calling LXSTAT() when we set the flag where your patch
 deleted lines, should result in the desired behavior, though, possibly
 with a test for !FTW_PHYS)
  

DJ Delorie <dj@redhat.com> writes:

> "Tulio Magno Quites Machado Filho" <tuliom@linux.vnet.ibm.com> writes:
>> POSIX.1-2008 doesn't describe this behavior explicitly in this case.
>
> See also https://bugzilla.redhat.com/show_bug.cgi?id=1422736
> and http://austingroupbugs.net/view.php?id=1121
>
> The spec also says "If FTW_PHYS is clear ... nftw() shall follow links
> instead of reporting them" but calling lstat() *is* reporting them,
> which the spec specifically forbids, if you read it that way.

Agreed.

> Hence, the clarification I requested from the Austin Group.  If the spec
> says we're not allowed to call lstat(), your patch is inappropriate.  If
> we are allowed to call lstat(), I think your patch is wrong - it may
> return FTW_NS for cases where lstat() might fail, where FTW_SLN should
> have been returned.  I can't think of any such cases, but I've been
> surprised before.  Also, cases where FTW_NS should be returned due to
> errors reading *non*dangling links should be preserved.

I think that's yet another interpretation of the spec.
So, we have to wait for their position.

> (I suspect calling LXSTAT() when we set the flag where your patch
>  deleted lines, should result in the desired behavior, though, possibly
>  with a test for !FTW_PHYS)

Ack.

Thanks for the references!