Re: [PATCH v2][next] tty: sunsu: Simplify device_node cleanup by using __free

[Shresth Prasad]

On Thu, May 2, 2024 at 1:26 PM Jiri Slaby <jirislaby@xxxxxxxxxx> 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@xxxxxxxx>
> > Signed-off-by: Shresth Prasad <shresthprasad7@xxxxxxxxx>
> > ---
> > 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