| Message ID | YMtY7nOtqEvTokh7@mwanda (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | [v2,1/2] clk: renesas: fix a double free on error | expand |
Hi Dan, Thank you for the patch. > -----Original Message----- > From: Dan Carpenter <dan.carpenter@oracle.com> > Sent: 17 June 2021 15:15 > To: Geert Uytterhoeven <geert+renesas@glider.be>; Prabhakar Mahadev Lad <prabhakar.mahadev- > lad.rj@bp.renesas.com> > Cc: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; linux-renesas- > soc@vger.kernel.org; linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; kernel- > janitors@vger.kernel.org > Subject: [PATCH 2/2] clk: renesas: Avoid mixing error pointers and NULL > > These functions accidentally return both error pointers and NULL when there is an error. It doesn't > cause a problem but it is confusing and seems unintentional. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/clk/renesas/renesas-rzg2l-cpg.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Cheers, Prabhakar > diff --git a/drivers/clk/renesas/renesas-rzg2l-cpg.c b/drivers/clk/renesas/renesas-rzg2l-cpg.c > index 7ba36f19896f..83b58e1cb78f 100644 > --- a/drivers/clk/renesas/renesas-rzg2l-cpg.c > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c > @@ -124,7 +124,7 @@ rzg2l_cpg_div_clk_register(const struct cpg_core_clk *core, > core->flag, &priv->rmw_lock); > > if (IS_ERR(clk_hw)) > - return NULL; > + return ERR_CAST(clk_hw); > > return clk_hw->clk; > } > @@ -174,17 +174,14 @@ rzg2l_cpg_pll_clk_register(const struct cpg_core_clk *core, > struct clk_init_data init; > const char *parent_name; > struct pll_clk *pll_clk; > - struct clk *clk; > > parent = clks[core->parent & 0xffff]; > if (IS_ERR(parent)) > return ERR_CAST(parent); > > pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL); > - if (!pll_clk) { > - clk = ERR_PTR(-ENOMEM); > - return NULL; > - } > + if (!pll_clk) > + return ERR_PTR(-ENOMEM); > > parent_name = __clk_get_name(parent); > init.name = core->name; > -- > 2.30.2
Hi Dan, On Thu, Jun 17, 2021 at 4:15 PM Dan Carpenter <dan.carpenter@oracle.com> wrote: > These functions accidentally return both error pointers and NULL when > there is an error. It doesn't cause a problem but it is confusing and > seems unintentional. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-clk-for-v5.15. > --- a/drivers/clk/renesas/renesas-rzg2l-cpg.c > +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c > @@ -124,7 +124,7 @@ rzg2l_cpg_div_clk_register(const struct cpg_core_clk *core, > core->flag, &priv->rmw_lock); > > if (IS_ERR(clk_hw)) > - return NULL; > + return ERR_CAST(clk_hw); > > return clk_hw->clk; > } > @@ -174,17 +174,14 @@ rzg2l_cpg_pll_clk_register(const struct cpg_core_clk *core, > struct clk_init_data init; > const char *parent_name; > struct pll_clk *pll_clk; > - struct clk *clk; > > parent = clks[core->parent & 0xffff]; > if (IS_ERR(parent)) > return ERR_CAST(parent); > > pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL); > - if (!pll_clk) { > - clk = ERR_PTR(-ENOMEM); > - return NULL; > - } > + if (!pll_clk) > + return ERR_PTR(-ENOMEM); This part I already have, by virtue of https://lore.kernel.org/r/1623896524-102058-1-git-send-email-yang.lee@linux.alibaba.com Gr{oetje,eeting}s, Geert
diff --git a/drivers/clk/renesas/renesas-rzg2l-cpg.c b/drivers/clk/renesas/renesas-rzg2l-cpg.c index 7ba36f19896f..83b58e1cb78f 100644 --- a/drivers/clk/renesas/renesas-rzg2l-cpg.c +++ b/drivers/clk/renesas/renesas-rzg2l-cpg.c @@ -124,7 +124,7 @@ rzg2l_cpg_div_clk_register(const struct cpg_core_clk *core, core->flag, &priv->rmw_lock); if (IS_ERR(clk_hw)) - return NULL; + return ERR_CAST(clk_hw); return clk_hw->clk; } @@ -174,17 +174,14 @@ rzg2l_cpg_pll_clk_register(const struct cpg_core_clk *core, struct clk_init_data init; const char *parent_name; struct pll_clk *pll_clk; - struct clk *clk; parent = clks[core->parent & 0xffff]; if (IS_ERR(parent)) return ERR_CAST(parent); pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL); - if (!pll_clk) { - clk = ERR_PTR(-ENOMEM); - return NULL; - } + if (!pll_clk) + return ERR_PTR(-ENOMEM); parent_name = __clk_get_name(parent); init.name = core->name;
These functions accidentally return both error pointers and NULL when there is an error. It doesn't cause a problem but it is confusing and seems unintentional. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/clk/renesas/renesas-rzg2l-cpg.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)