Received-SPF: pass (google.com: domain of linux-kernel+bounces-133009-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>
In-Reply-To: <CAGETcx_H_vvK9y-51JTcz8F7GDThBwC+t=k2i6r4Nst3H6-TUg@mail.gmail.com>
From: Rob Herring <robh@kernel.org>
Date: Fri, 5 Apr 2024 07:43:56 -0500
Message-ID: <CAL_Jsq+eS3uS1rEPSErMnfJhPBoQjh7UyRLa9NZ59WBVcD+JWw@mail.gmail.com>
Subject: Re: [PATCH 2/3] of: Use scope based kfree() cleanups
To: Saravana Kannan <saravanak@google.com>
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 Thu, Apr 4, 2024 at 6:16=E2=80=AFPM Saravana Kannan <saravanak@google.co=
m> wrote:
>
> On Thu, Apr 4, 2024 at 7:15=E2=80=AFAM Rob Herring <robh@kernel.org> wrot=
e:
> >
> > 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 struct =
device_node *np,
> >                                    const char *stem_name,
> >                                    int index, struct of_phandle_args *o=
ut_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-c=
ells", stem_name);
> > +       char *map_name __free(kfree) =3D kasprintf(GFP_KERNEL, "%s-map"=
, stem_name);
> > +       char *mask_name __free(kfree) =3D kasprintf(GFP_KERNEL, "%s-map=
-mask", stem_name);
> > +       char *pass_name __free(kfree) =3D kasprintf(GFP_KERNEL, "%s-map=
-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.

Rob