Re: [PATCH] fchmodat/fstatat: fix regression with empty `pathname`

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

Hi Johannes,

On Jul  4 20:38, Corinna Vinschen wrote:
> On Jul  4 17:45, Johannes Schindelin wrote:
> > [...]
> > BTW a colleague and I were wondering whether we really want to set
> > `errno=ENOTDIR` in `gen_full_path_at()` for empty paths when
> > `AT_EMPTY_PATH` is _not_ specified. As far as we can tell, Linux sets
> > `errno=ENOENT` in that instance.
> 
> I wonder if that's really what you mean.  gen_full_path_at() generates
> ENOTDIR in two scenarios:
> 
> - At line 4443, if Cygwin can't resolve dirfd into a valid directory.
> 
> - At line 4450 if ... actually... never.  Given that p is always
>   set to the end of the directory string copied into path_ret, it
>   can never be NULL. Looks like this check for !p is a remnant from
>   the past.  We should remove it.
> 
> The actual check for an empty path is done in line 4457, and this
> results in ENOENT, as desired.
> 
> So, by any chance, do you mean the situation handled in line 4443,
> that is, returning ENOTDIR if dirfd doesn't resolve to a directory?
> 
> Yeah, it slightly complicates the caller, but it's not exactly
> wrong, given your patch.
> 
> OTOH, this entire thing doesn't look overly well thought out.  We try
> to generate a full path in gen_full_path_at() and if it fails in
> a certain way and AT_EMPTY_PATH is given, we basically repeat
> trying to create a full path in the caller.  Maybe some
> streamlining would be in order...

I actually found some time, to do that.  So I now have a counter
proposal to your patch.  I'll send the patch series in a minute.  Would
you mind to take a discerning look and, perhaps, give it a try, too?


Thanks,
Corinna