linux-kernel - Re: [PATCH v2][next] tty: sunsu: Simplify device_node cleanup by using _

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]

Date: Thu, 2 May 2024 14:25:14 +0530
From: Shresth Prasad <shresthprasad7@...il.com>
To: Jiri Slaby <jirislaby@...nel.org>
Cc: davem@...emloft.net, gregkh@...uxfoundation.org, 
	sparclinux@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org, javier.carrasco.cruz@...il.com, 
	skhan@...uxfoundation.org, Julia Lawall <julia.lawall@...ia.fr>
Subject: Re: [PATCH v2][next] tty: sunsu: Simplify device_node cleanup by
 using __free

On Thu, May 2, 2024 at 1:26 PM Jiri Slaby <jirislaby@...nel.org> wrote:
>
> On 01. 05. 24, 10:41, Shresth Prasad wrote:
> > Add `__free` function attribute to `ap` and `match` pointer
> > initialisations which ensure that the pointers are freed as soon as they
> > go out of scope, thus removing the need to manually free them using
> > `of_node_put`.
> >
> > This also removes the need for the `goto` statement and the `rc`
> > variable.
> >
> > Tested using a qemu x86_64 virtual machine.
> >
> > Suggested-by: Julia Lawall <julia.lawall@...ia.fr>
> > Signed-off-by: Shresth Prasad <shresthprasad7@...il.com>
> > ---
> > Changes in v2:
> >      - Specify how the patch was tested
> >
> >   drivers/tty/serial/sunsu.c | 37 +++++++++++--------------------------
> >   1 file changed, 11 insertions(+), 26 deletions(-)
>
> Nice cleanup.

Thanks :)

>
> > --- a/drivers/tty/serial/sunsu.c
> > +++ b/drivers/tty/serial/sunsu.c
> > @@ -1382,44 +1382,29 @@ static inline struct console *SUNSU_CONSOLE(void)
> >
> >   static enum su_type su_get_type(struct device_node *dp)
> >   {
> > -     struct device_node *ap = of_find_node_by_path("/aliases");
> > -     enum su_type rc = SU_PORT_PORT;
> > +     struct device_node *ap __free(device_node) =
> > +                         of_find_node_by_path("/aliases");
>
> If we used c++, that would be even nicer; like:
> Destroyer ap(of_find_node_by_path("/aliases"));
>
> But we don't :P. OTOH. can't we achieve that with macro-fu and typeof()
> magic? Perhaps not really exactly the above, but something like
>    Destroyer(ap, of_find_node_by_path("/aliases"));
> maybe?

Hmm, a macro like that could probably be written but it's more convenient
to use the GCC attribute like in the current implementation, IMO.

Jonathan Corbert, who introduced this, wrote about it here:
https://lwn.net/Articles/934679/

Regards,
Shresth