lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <878qhpc3ip.fsf@>
Date: Sun, 05 Oct 2025 07:37:50 +0200
From: Miquel Sabaté Solà <mssola@...ola.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: linux-fsdevel@...r.kernel.org,  brauner@...nel.org,
  linux-kernel@...r.kernel.org,  jack@...e.cz
Subject: Re: [PATCH] fs: Use a cleanup attribute in copy_fdtable()

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@...iv.linux.org.uk>

Thanks for the review,
Miquel

Download attachment "signature.asc" of type "application/pgp-signature" (898 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ