Re: [patch V2 01/10] cleanup: Provide retain_ptr()

Peter Zijlstra
2025-03-14T09:37:33Z

On Thu, Mar 13, 2025 at 02:03:38PM +0100, Thomas Gleixner wrote:
> In cases where an allocation is consumed by another function, the
> allocation needs to be retained on success or freed on failure. The code
> pattern is usually:
>
> struct foo *f = kzalloc(sizeof(*f), GFP_KERNEL);
> struct bar *b;
>
> ,,,
> // Initialize f
> ...
> if (ret)
> goto free;
> ...
> bar = bar_create(f);
> if (!bar) {
> ret = -ENOMEM;
> goto free;
> }
> ...
> return 0;
> free:
> kfree(f);
> return ret;
>
> This prevents using __free(kfree) on @f because there is no canonical way
> to tell the cleanup code that the allocation should not be freed.
>
> Abusing no_free_ptr() by force ignoring the return value is not really a
> sensible option either.
>
> Provide an explicit macro retain_ptr(), which NULLs the cleanup
> pointer. That makes it easy to analyze and reason about.

So no objection per se, but one way to solve this is by handing
ownership to bar_create(), such that it is responsible for freeing f on
failure.

Anyway, I suspect the __must_check came from Linus, OTOH take_fd(), the
equivalent for file descriptors also don't have that __must_check. So
clearly we have precedent here.