[PATCH] fchmodat/fstatat: fix regression with empty `pathname`
Corinna Vinschen
corinna-cygwin@cygwin.com
Wed Jul 12 11:50:15 GMT 2023
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
More information about the Cygwin-patches
mailing list