Re: [PATCH] fs: Use a cleanup attribute in copy_fdtable()

["Miquel Sabaté Solà" <mssola@mssola.com>]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Al Viro @ 2025-10-04 22:19 +01:

> On Sat, Oct 04, 2025 at 11:03:40PM +0200, Miquel Sabaté Solà wrote:
>> This is a small cleanup in which by using the __free(kfree) cleanup
>> attribute we can avoid three labels to go to, and the code turns to be
>> more concise and easier to follow.
>
> Have you tried to build and boot that?

Yes, and it worked on my machine...

>
> That aside, it is not easier to follow in that form - especially since
> kfree() is *not* the right destructor for the object in question.
> Having part of destructor done via sodding __cleanup, with the rest
> open-coded on various failure exits is confusing as hell.
>
> RAII has its uses, but applied unidiomatically it ends up being a mess
> that is harder to follow and reason about than the dreadful gotos it
> replaces.

I agree that it would generally not be the right destructor for it, but
in the case of this function it ends up being equivalent. But I see
that, if in general that wouldn't be the proper way, declaring the
fdtable variable like that can be misleading, even if equivalent
here. Thus, defeating the purpose of this patch.

>
> NAKed-by: Al Viro <viro@zeniv.linux.org.uk>

Thanks for the review,
Miquel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 897 bytes --]