Re: [PATCH 2/2] btrfs: more trivial BTRFS_PATH_AUTO_FREE conversions

David Sterba
2025-08-26T16:56:22Z

On Fri, Aug 22, 2025 at 11:51:18PM +0800, Sun YangKai wrote:
> > Please split the patch to parts that have the described trivial changes,
> > and then one patch per function in case it's not trivial and needs some
> > adjustments.
>
> After learning more about the auto-free/cleanup mechanism, I realized that its
> only advantage is to eliminate the need for the goto out; pattern. Therefore,
> it seems unnecessary to apply this conversion in non-trivial cases.

I wouldn't say it's the only advantage, the code readability is also
improved. The path is an auxiliary object and if the freeing is handled
automatically then it reduces the cognitive load and the error cleanup
paths.

> Moreover, if the cleanup code contains other logic, it might be better to
> leave it unchanged even in trivial cases.

Depends on what we want. So far we've started with the path auto
cleaning but there are more possibilities like using the raw __free
cleanup with kfree. If this is combined and leads to simpler exit and
cleanup blocks I think it's worth. In the trivial cases it's clear it
does not interfere with the rest of the code and does not complicate any
logic there.

> > The freeing followed by other code can be still converted to auto
> > cleaning but there must be an explicit path = NULL after the free.
>
> I'm sorry, I didn't understand. If the freeing is followed by other code,
> maybe we could just leave them untouched?

Maybe yes, this is up to consideration on a per site basis, I've seen
examples where conversion to auto path cleaning would not hurt.

Let me know if you want to continue with this because I think you don't
seem to see the value in the conversions (which is fine of course).