On Mon, Jul 12, 2021 at 5:37 AM Dmitry Kadashev <dkadashev@xxxxxxxxx> wrote:
>
> +
> +int do_symlinkat(struct filename *from, int newdfd, struct filename *to)
> +{
> + int error;
> +
> + if (IS_ERR(from)) {
> + error = PTR_ERR(from);
> + goto out;
> }
> -out_putnames:
> +
> + error = symlinkat_helper(from, newdfd, to, 0);
> + if (retry_estale(error, 0))
> + error = symlinkat_helper(from, newdfd, to, LOOKUP_REVAL);
> +
> +out:
> putname(to);
> putname(from);
> return error;
So here you moved that part that was outside the retry loop into the
caller. Except it's very ugly and keeps the goto mess.
So I'd suggest either keep it as a nested if - avoiding the goto - or
like in the previous patch, do that "we can do this test twice" with a
big commit message note about why it's ok.
Because it _is_ ok to repeat the test inside the retry_estale, and
'from' won't have changed (and won't have been -ESTALE in the first
place).
Looking at the pattern of this and the previous one, I think just
repeating the test is what generates the cleanest end result.
Linus
