* Re: Fwd: [PATCH] irqchip:Use the "put_device" function to release the memory after calling the of_find_device_by_node function.
2023-07-14 3:43 ` Fwd: [PATCH] irqchip:Use the "put_device" function to release the memory after calling the of_find_device_by_node function liubin001
@ 2023-07-14 8:06 ` Marc Zyngier
0 siblings, 0 replies; 2+ messages in thread
From: Marc Zyngier @ 2023-07-14 8:06 UTC (permalink / raw)
To: liubin001; +Cc: tglx, linux-kernel, Geert Uytterhoeven
Please don't forward emails like this. If you've missed a recipient,
repost it.
+Geert for the Renesas stuff.
On Fri, 14 Jul 2023 04:43:28 +0100,
liubin001@208suo.com wrote:
>
> Write the put_device function before return
Why? What is wrong with the current code? What does it affect?
>
> Signed-off-by: Liu Bin <liubin001@208suo.com>
> ---
> drivers/irqchip/irq-renesas-rzg2l.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> b/drivers/irqchip/irq-renesas-rzg2l.c
> index 4bbfa2b0a4df..a9d75135ba24 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -325,35 +325,42 @@ static int rzg2l_irqc_init(struct device_node
> *node, struct device_node *parent)
>
> pdev = of_find_device_by_node(node);
> if (!pdev)
> + put_device(pdev);
> return -ENODEV;
>
> parent_domain = irq_find_host(parent);
> if (!parent_domain) {
> dev_err(&pdev->dev, "cannot find parent domain\n");
> + put_device(pdev);
> return -ENODEV;
> }
>
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> + put_device(pdev);
> return -ENOMEM;
>
> priv->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
> if (IS_ERR(priv->base))
> + put_device(pdev);
> return PTR_ERR(priv->base);
>
> ret = rzg2l_irqc_parse_interrupts(priv, node);
> if (ret) {
> dev_err(&pdev->dev, "cannot parse interrupts: %d\n", ret);
> + put_device(pdev);
> return ret;
> }
>
> resetn = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> if (IS_ERR(resetn))
> + put_device(pdev);
> return PTR_ERR(resetn);
>
> ret = reset_control_deassert(resetn);
> if (ret) {
> dev_err(&pdev->dev, "failed to deassert resetn pin, %d\n",
> ret);
> + put_device(pdev);
> return ret;
> }
>
> @@ -374,7 +381,7 @@ static int rzg2l_irqc_init(struct device_node
> *node, struct device_node *parent)
> ret = -ENOMEM;
> goto pm_put;
> }
> -
> + put_device(pdev);
> return 0;
>
> pm_put:
> @@ -382,6 +389,7 @@ static int rzg2l_irqc_init(struct device_node
> *node, struct device_node *parent)
> pm_disable:
> pm_runtime_disable(&pdev->dev);
> reset_control_assert(resetn);
> + put_device(pdev);
> return ret;
> }
Can't you spot an easy way to make the whole thing more readable and
less error prone? Such as having a *single* exit point to the
function, doing the put_device() and returning the error code?
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 2+ messages in thread