Received-SPF: pass (google.com: domain of linux-kernel+bounces-133513-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240404-dt-cleanup-free-v1-0-c60e6cba8da9@kernel.org>
 <20240404-dt-cleanup-free-v1-2-c60e6cba8da9@kernel.org> <CAGETcx_H_vvK9y-51JTcz8F7GDThBwC+t=k2i6r4Nst3H6-TUg@mail.gmail.com>
 <CAL_Jsq+eS3uS1rEPSErMnfJhPBoQjh7UyRLa9NZ59WBVcD+JWw@mail.gmail.com>
In-Reply-To: <CAL_Jsq+eS3uS1rEPSErMnfJhPBoQjh7UyRLa9NZ59WBVcD+JWw@mail.gmail.com>
From: Saravana Kannan <saravanak@google.com>
Date: Fri, 5 Apr 2024 12:12:38 -0700
Message-ID: <CAGETcx9-6KCLJh0gYQwxV5cAJLvgo1fpwzuxH51AHVLkDS0w_g@mail.gmail.com>
Subject: Re: [PATCH 2/3] of: Use scope based kfree() cleanups
To: Rob Herring <robh@kernel.org>
Cc: Jonathan Cameron <jic23@kernel.org>, devicetree@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 5, 2024 at 5:44=E2=80=AFAM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Apr 4, 2024 at 6:16=E2=80=AFPM Saravana Kannan <saravanak@google.=
com> wrote:
> >
> > On Thu, Apr 4, 2024 at 7:15=E2=80=AFAM Rob Herring <robh@kernel.org> wr=
ote:
> > >
> > > Use the relatively new scope based kfree() cleanup to simplify error
> > > handling. Doing so reduces the chances of memory leaks and simplifies
> > > error paths by avoiding the need for goto statements.
> > >
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  drivers/of/base.c     | 34 ++++++++--------------------------
> > >  drivers/of/dynamic.c  | 11 ++++-------
> > >  drivers/of/resolver.c | 35 +++++++++++++----------------------
> > >  3 files changed, 25 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > > index 8856c67c466a..20603d3c9931 100644
> > > --- a/drivers/of/base.c
> > > +++ b/drivers/of/base.c
> > > @@ -16,6 +16,7 @@
> > >
> > >  #define pr_fmt(fmt)    "OF: " fmt
> > >
> > > +#include <linux/cleanup.h>
> > >  #include <linux/console.h>
> > >  #include <linux/ctype.h>
> > >  #include <linux/cpu.h>
> > > @@ -1393,8 +1394,10 @@ int of_parse_phandle_with_args_map(const struc=
t device_node *np,
> > >                                    const char *stem_name,
> > >                                    int index, struct of_phandle_args =
*out_args)
> > >  {
> > > -       char *cells_name, *map_name =3D NULL, *mask_name =3D NULL;
> > > -       char *pass_name =3D NULL;
> > > +       char *cells_name __free(kfree) =3D kasprintf(GFP_KERNEL, "#%s=
-cells", stem_name);
> > > +       char *map_name __free(kfree) =3D kasprintf(GFP_KERNEL, "%s-ma=
p", stem_name);
> > > +       char *mask_name __free(kfree) =3D kasprintf(GFP_KERNEL, "%s-m=
ap-mask", stem_name);
> > > +       char *pass_name __free(kfree) =3D kasprintf(GFP_KERNEL, "%s-m=
ap-pass-thru", stem_name);
> >
> > With the scoped stuff, do these function calls need to be in the same
> > line we are defining these variables? If not, I'd rather that the
> > calls remain where they were. It feels like a lote to visually parse
> > and take in from a readability perspective.
>
> They don't have to be, but if you don't want to get yelled at by the
> chief penguin, then yes, they should be together. See the discussions
> on adding the scoped iterators. But with the C99 adoption, we can move
> the declaration to where the assignment was original.

Thanks for the context and the link in the other email.

Review-by without reservations.

-Saravana