linux-kernel - Re: [PATCH 2/3] of: Use scope based kfree() cleanups

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 5 Apr 2024 07:43:56 -0500
From: Rob Herring <robh@...nel.org>
To: Saravana Kannan <saravanak@...gle.com>
Cc: Jonathan Cameron <jic23@...nel.org>, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] of: Use scope based kfree() cleanups

On Thu, Apr 4, 2024 at 6:16 PM Saravana Kannan <saravanak@...gle.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@...nel.org> wrote:
> >
> > 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@...nel.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 *out_args)
> >  {
> > -       char *cells_name, *map_name = NULL, *mask_name = NULL;
> > -       char *pass_name = NULL;
> > +       char *cells_name __free(kfree) = kasprintf(GFP_KERNEL, "#%s-cells", stem_name);
> > +       char *map_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map", stem_name);
> > +       char *mask_name __free(kfree) = kasprintf(GFP_KERNEL, "%s-map-mask", stem_name);
> > +       char *pass_name __free(kfree) = 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