On Fri, Apr 5, 2024 at 5:44 AM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> 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.
Thanks for the context and the link in the other email.
Review-by without reservations.
-Saravana
