Re: [PATCH v8 02/11] namei: change filename_parentat() calling conventions

Linus Torvalds
2021-07-07T19:18:47Z

On Wed, Jul 7, 2021 at 5:28 AM Dmitry Kadashev <dkadashev@gmail.com> 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