On 25/03/2023 at 15:05, Markus Elfring wrote:
Date: Fri, 17 Mar 2023 20:02:34 +0100
The label “err_free” was used to jump to another pointer check despite of
the detail in the implementation of the function “sama7g5_pmc_setup”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed memory allocation).
* Thus use additional labels.
* Delete an extra pointer check at the end which became unnecessary
with this refactoring.
This issue was detected by using the Coccinelle software.
Fine, but I'm sorry that it complexity the function for no real value.
Other clk drivers have the same pattern so I want them to all stay the same.
This is a NACK, sorry about that.
Regards,
Nicolas
Fixes: cb783bbbcf54c36256006895c215e86c5e7266d8 ("clk: at91: sama7g5: add clock support for sama7g5")
Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/clk/at91/sama7g5.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
index f135b662f1ff..224b1f2ebef2 100644
--- a/drivers/clk/at91/sama7g5.c
+++ b/drivers/clk/at91/sama7g5.c
@@ -927,25 +927,25 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
(ARRAY_SIZE(sama7g5_mckx) + ARRAY_SIZE(sama7g5_gck)),
GFP_KERNEL);
if (!alloc_mem)
- goto err_free;
+ goto free_pmc;
hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000,
50000000);
if (IS_ERR(hw))
- goto err_free;
+ goto free_alloc_mem;
bypass = of_property_read_bool(np, "atmel,osc-bypass");
hw = at91_clk_register_main_osc(regmap, "main_osc", mainxtal_name,
bypass);
if (IS_ERR(hw))
- goto err_free;
+ goto free_alloc_mem;
parent_names[0] = "main_rc_osc";
parent_names[1] = "main_osc";
hw = at91_clk_register_sam9x5_main(regmap, "mainck", parent_names, 2);
if (IS_ERR(hw))
- goto err_free;
+ goto free_alloc_mem;
sama7g5_pmc->chws[PMC_MAIN] = hw;
@@ -987,7 +987,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
}
if (IS_ERR(hw))
- goto err_free;
+ goto free_alloc_mem;
if (sama7g5_plls[i][j].eid)
sama7g5_pmc->chws[sama7g5_plls[i][j].eid] = hw;
@@ -999,7 +999,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
&mck0_layout, &mck0_characteristics,
&pmc_mck0_lock, CLK_GET_RATE_NOCACHE, 5);
if (IS_ERR(hw))
- goto err_free;
+ goto free_alloc_mem;
sama7g5_pmc->chws[PMC_MCK] = hw;
@@ -1128,12 +1128,11 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
return;
err_free:
- if (alloc_mem) {
- for (i = 0; i < alloc_mem_size; i++)
- kfree(alloc_mem[i]);
- kfree(alloc_mem);
- }
-
+ for (i = 0; i < alloc_mem_size; i++)
+ kfree(alloc_mem[i]);
+free_alloc_mem:
+ kfree(alloc_mem);
+free_pmc:
kfree(sama7g5_pmc);
}
--
2.40.0
--
Nicolas Ferre
