Re: [PATCH v4 2/3] device property: Refactor to use RAII approach

[Rafael J. Wysocki]

On Thu, Jun 11, 2026 at 10:35 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> In a couple of functions code can be made cleaner with help of
> __free() macro. Refactor these to use RAII approach.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/base/property.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index f7b30d9c8716..808e8a90c125 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -7,10 +7,10 @@
>   *          Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
>   */
>
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/export.h>
> -#include <linux/kconfig.h>
>  #include <linux/of.h>
>  #include <linux/property.h>
>  #include <linux/phy.h>
> @@ -517,7 +517,6 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_string);
>  int fwnode_property_match_string(const struct fwnode_handle *fwnode,
>         const char *propname, const char *string)
>  {
> -       const char **values;
>         int nval, ret;
>
>         nval = fwnode_property_string_array_count(fwnode, propname);
> @@ -527,20 +526,18 @@ int fwnode_property_match_string(const struct fwnode_handle *fwnode,
>         if (nval == 0)
>                 return -ENODATA;
>
> -       values = kcalloc(nval, sizeof(*values), GFP_KERNEL);
> +       const char **values __free(kfree) = kcalloc(nval, sizeof(*values), GFP_KERNEL);
>         if (!values)
>                 return -ENOMEM;
>
>         ret = fwnode_property_read_string_array(fwnode, propname, values, nval);
>         if (ret < 0)
> -               goto out_free;
> +               return ret;
>
>         ret = match_string(values, nval, string);
>         if (ret < 0)
> -               ret = -ENODATA;
> +               return -ENODATA;
>
> -out_free:
> -       kfree(values);
>         return ret;
>  }
>  EXPORT_SYMBOL_GPL(fwnode_property_match_string);
> @@ -1128,8 +1125,9 @@ struct fwnode_handle *
>  fwnode_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
>                                struct fwnode_handle *prev)
>  {
> -       struct fwnode_handle *ep, *port_parent = NULL;
>         const struct fwnode_handle *parent;
> +       struct fwnode_handle *port_parent __free(fwnode_handle) = NULL;

The thing on the right-hand side of the assignment should be a
constructor, shouldn't it?

> +       struct fwnode_handle *ep;
>
>         /*
>          * If this function is in a loop and the previous iteration returned
> @@ -1147,13 +1145,9 @@ fwnode_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
>
>         ep = fwnode_call_ptr_op(parent, graph_get_next_endpoint, prev);
>         if (ep)
> -               goto out_put_port_parent;
> +               return ep;
>
> -       ep = fwnode_graph_get_next_endpoint(parent->secondary, NULL);
> -
> -out_put_port_parent:
> -       fwnode_handle_put(port_parent);
> -       return ep;
> +       return fwnode_graph_get_next_endpoint(parent->secondary, NULL);
>  }
>  EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);
>
> --
> 2.50.1
>