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

[Rob Herring]

On Thu, Apr 4, 2024 at 6:16 PM Saravana Kannan <saravanak@xxxxxxxxxx> wrote:
>
> On Thu, Apr 4, 2024 at 7:15 AM Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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