On Wed, Jul 7, 2021 at 5:28 AM Dmitry Kadashev <dkadashev@xxxxxxxxx> wrote:
>
> Hence this preparation change splits filename_parentat() into two: one
> that always consumes the name and another that never consumes the name.
> This will allow to implement two filename_create() variants in the same
> way, and is a consistent and hopefully easier to reason about approach.
I like it.
The patch itself is a bit hard to read, but the end result seems to make sense.
My main reaction is that this could have probably done a bit more
cleanup by avoiding some of the "goto exit1" kind of things.
Just as an example, now the rule is that "do_rmdir()" always does that
> putname(name);
> return error;
at the end, and I think this means that this whole function could be
split into a few trivial helper functions instead, and we'd have
long do_rmdir(int dfd, struct filename *name)
{
int error;
error = rmdir_helper(...)
if (!retry_estale(error, lookup_flags)) {
lookup_flags |= LOOKUP_REVAL;
error = rmdir_helper(...);
}
putname(name);
return error;
}
which gets rid of both the "goto retry" and the "goto exit1".
With the meat of "do_rmdir()" done in that "rmdir_helper()" function.
I think the same is basically true of "do_unlinkat()" too.
But I wouldn't mind that cleanup as a separate patch. My point is that
I think this new rule for when the name is consumed is better and can
result in further cleanups.
(NOTE! This is from just reading the patch, I might have missed some case).
Linus
