Received-SPF: pass (google.com: domain of linux-kernel+bounces-166160-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240501084110.4165-2-shresthprasad7@gmail.com> <01cbcdf5-8a7a-4b47-a2aa-05c041e3cab2@kernel.org>
In-Reply-To: <01cbcdf5-8a7a-4b47-a2aa-05c041e3cab2@kernel.org>
From: Shresth Prasad <shresthprasad7@gmail.com>
Date: Thu, 2 May 2024 14:25:14 +0530
Message-ID: <CAE8VWiKqnM0BGnSDxT5+ZjD67UXxjY3PZAyNwsxbD0nZs3cJ4A@mail.gmail.com>
Subject: Re: [PATCH v2][next] tty: sunsu: Simplify device_node cleanup by
 using __free
To: Jiri Slaby <jirislaby@kernel.org>
Cc: davem@davemloft.net, gregkh@linuxfoundation.org, 
	sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-serial@vger.kernel.org, javier.carrasco.cruz@gmail.com, 
	skhan@linuxfoundation.org, Julia Lawall <julia.lawall@inria.fr>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, May 2, 2024 at 1:26=E2=80=AFPM Jiri Slaby <jirislaby@kernel.org> wr=
ote:
>
> 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 the=
y
> > 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@inria.fr>
> > Signed-off-by: Shresth Prasad <shresthprasad7@gmail.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(voi=
d)
> >
> >   static enum su_type su_get_type(struct device_node *dp)
> >   {
> > -     struct device_node *ap =3D of_find_node_by_path("/aliases");
> > -     enum su_type rc =3D SU_PORT_PORT;
> > +     struct device_node *ap __free(device_node) =3D
> > +                         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