From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 17:03:21 +0100
Several update suggestions were taken into account
from static source code analysis.
Markus Elfring (10):
Less function calls in of_omap2_apll_setup() after error detection
Less function calls in of_dra7_apll_setup() after error detection
Use common code in omap_clk_register_apll()
Less function calls in ti_fapll_setup() after error detection
One function call less in ti_fapll_synth_setup() after error detection
Return directly after a failed kzalloc() in of_mux_clk_setup()
Less function calls in _ti_omap4_clkctrl_setup() after error detection
Use common error handling code in _ti_omap4_clkctrl_setup()
Less function calls in _ti_clkctrl_clk_register() after error detection
Delete an unnecessary initialisation in _ti_clkctrl_clk_register()
drivers/clk/ti/apll.c | 58 ++++++++++++++++++++++++----------------
drivers/clk/ti/clkctrl.c | 44 ++++++++++++++++--------------
drivers/clk/ti/fapll.c | 29 +++++++++++---------
drivers/clk/ti/mux.c | 2 +-
4 files changed, 76 insertions(+), 57 deletions(-)
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 11:15:27 +0100
The kfree() function was called in up to three cases by
the of_omap2_apll_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
* Split a condition check.
* Adjust jump targets.
* Delete three initialisations which became unnecessary
with this refactoring.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/apll.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index 93183287c58d..929c021f0cdb 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -338,21 +338,25 @@ static const struct clk_hw_omap_ops omap2_apll_hwops = {
static void __init of_omap2_apll_setup(struct device_node *node)
{
- struct dpll_data *ad = NULL;
- struct clk_hw_omap *clk_hw = NULL;
- struct clk_init_data *init = NULL;
+ struct dpll_data *ad;
+ struct clk_hw_omap *clk_hw;
+ struct clk_init_data *init = kzalloc(sizeof(*init), GFP_KERNEL);
const char *name;
struct clk *clk;
const char *parent_name;
u32 val;
int ret;
- ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+ if (!init)
+ return;
+
clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
- init = kzalloc(sizeof(*init), GFP_KERNEL);
+ if (!clk_hw)
+ goto free_init;
- if (!ad || !clk_hw || !init)
- goto cleanup;
+ ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+ if (!ad)
+ goto free_clk_hw;
clk_hw->dpll_data = ad;
clk_hw->hw.init = init;
@@ -403,12 +407,13 @@ static void __init of_omap2_apll_setup(struct device_node *node)
clk = of_ti_clk_register_omap_hw(node, &clk_hw->hw, name);
if (!IS_ERR(clk)) {
of_clk_add_provider(node, of_clk_src_simple_get, clk);
- kfree(init);
- return;
+ goto free_init;
}
cleanup:
kfree(ad);
+free_clk_hw:
kfree(clk_hw);
+free_init:
kfree(init);
}
CLK_OF_DECLARE(omap2_apll_clock, "ti,omap2-apll-clock",
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 12:27:09 +0100
The kfree() function was called in up to four cases by
the of_dra7_apll_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
* Split a condition check.
* Adjust jump targets.
* Delete four initialisations which became unnecessary
with this refactoring.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/apll.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index 929c021f0cdb..d2be672521a3 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -177,17 +177,22 @@ static void __init omap_clk_register_apll(void *user,
static void __init of_dra7_apll_setup(struct device_node *node)
{
- struct dpll_data *ad = NULL;
- struct clk_hw_omap *clk_hw = NULL;
- struct clk_init_data *init = NULL;
- const char **parent_names = NULL;
+ struct dpll_data *ad;
+ struct clk_hw_omap *clk_hw;
+ struct clk_init_data *init = kzalloc(sizeof(*init), GFP_KERNEL);
+ const char **parent_names;
int ret;
- ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+ if (!init)
+ return;
+
clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
- init = kzalloc(sizeof(*init), GFP_KERNEL);
- if (!ad || !clk_hw || !init)
- goto cleanup;
+ if (!clk_hw)
+ goto free_init;
+
+ ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+ if (!ad)
+ goto free_clk_hw;
clk_hw->dpll_data = ad;
clk_hw->hw.init = init;
@@ -198,12 +203,12 @@ static void __init of_dra7_apll_setup(struct device_node *node)
init->num_parents = of_clk_get_parent_count(node);
if (init->num_parents < 1) {
pr_err("dra7 apll %pOFn must have parent(s)\n", node);
- goto cleanup;
+ goto free_ad;
}
parent_names = kcalloc(init->num_parents, sizeof(char *), GFP_KERNEL);
if (!parent_names)
- goto cleanup;
+ goto free_ad;
of_clk_parent_fill(node, parent_names, init->num_parents);
@@ -223,8 +228,11 @@ static void __init of_dra7_apll_setup(struct device_node *node)
cleanup:
kfree(parent_names);
+free_ad:
kfree(ad);
+free_clk_hw:
kfree(clk_hw);
+free_init:
kfree(init);
}
CLK_OF_DECLARE(dra7_apll_clock, "ti,dra7-apll-clock", of_dra7_apll_setup);
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 12:40:59 +0100
Use another label so that two function calls can be better reused
at the end of this function.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/apll.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/ti/apll.c b/drivers/clk/ti/apll.c
index d2be672521a3..406326883741 100644
--- a/drivers/clk/ti/apll.c
+++ b/drivers/clk/ti/apll.c
@@ -163,16 +163,15 @@ static void __init omap_clk_register_apll(void *user,
clk = of_ti_clk_register_omap_hw(node, &clk_hw->hw, name);
if (!IS_ERR(clk)) {
of_clk_add_provider(node, of_clk_src_simple_get, clk);
- kfree(init->parent_names);
- kfree(init);
- return;
+ goto free_names;
}
cleanup:
kfree(clk_hw->dpll_data);
+ kfree(clk_hw);
+free_names:
kfree(init->parent_names);
kfree(init);
- kfree(clk_hw);
}
static void __init of_dra7_apll_setup(struct device_node *node)
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 13:55:44 +0100
The kfree() function was called in some cases by
the ti_fapll_setup() function during error handling
even if the passed variable contained a null pointer.
* Adjust jump targets.
* Delete two condition checks which became unnecessary
with this refactoring.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/fapll.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/ti/fapll.c b/drivers/clk/ti/fapll.c
index 2db3fc4a443e..e9956e3ccd65 100644
--- a/drivers/clk/ti/fapll.c
+++ b/drivers/clk/ti/fapll.c
@@ -546,11 +546,11 @@ static void __init ti_fapll_setup(struct device_node *node)
MAX_FAPLL_OUTPUTS + 1,
GFP_KERNEL);
if (!fd->outputs.clks)
- goto free;
+ goto free_fd;
init = kzalloc(sizeof(*init), GFP_KERNEL);
if (!init)
- goto free;
+ goto free_clks;
init->ops = &ti_fapll_ops;
name = ti_dt_clk_name(node);
@@ -559,7 +559,7 @@ static void __init ti_fapll_setup(struct device_node *node)
init->num_parents = of_clk_get_parent_count(node);
if (init->num_parents != 2) {
pr_err("%pOFn must have two parents\n", node);
- goto free;
+ goto free_init;
}
of_clk_parent_fill(node, parent_name, 2);
@@ -568,19 +568,19 @@ static void __init ti_fapll_setup(struct device_node *node)
fd->clk_ref = of_clk_get(node, 0);
if (IS_ERR(fd->clk_ref)) {
pr_err("%pOFn could not get clk_ref\n", node);
- goto free;
+ goto free_init;
}
fd->clk_bypass = of_clk_get(node, 1);
if (IS_ERR(fd->clk_bypass)) {
pr_err("%pOFn could not get clk_bypass\n", node);
- goto free;
+ goto put_clk_ref;
}
fd->base = of_iomap(node, 0);
if (!fd->base) {
pr_err("%pOFn could not get IO base\n", node);
- goto free;
+ goto put_clk_bypass;
}
if (fapll_is_ddr_pll(fd->base))
@@ -653,14 +653,16 @@ static void __init ti_fapll_setup(struct device_node *node)
unmap:
iounmap(fd->base);
-free:
- if (fd->clk_bypass)
- clk_put(fd->clk_bypass);
- if (fd->clk_ref)
- clk_put(fd->clk_ref);
+put_clk_bypass:
+ clk_put(fd->clk_bypass);
+put_clk_ref:
+ clk_put(fd->clk_ref);
+free_init:
+ kfree(init);
+free_clks:
kfree(fd->outputs.clks);
+free_fd:
kfree(fd);
- kfree(init);
}
CLK_OF_DECLARE(ti_fapll_clock, "ti,dm816-fapll-clock", ti_fapll_setup);
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 14:05:41 +0100
The kfree() function was called in one case by
the ti_fapll_synth_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus use another label.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/fapll.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/ti/fapll.c b/drivers/clk/ti/fapll.c
index e9956e3ccd65..d4674ec3d7e9 100644
--- a/drivers/clk/ti/fapll.c
+++ b/drivers/clk/ti/fapll.c
@@ -504,7 +504,7 @@ static struct clk * __init ti_fapll_synth_setup(struct fapll_data *fd,
synth = kzalloc(sizeof(*synth), GFP_KERNEL);
if (!synth)
- goto free;
+ goto free_init;
synth->fd = fd;
synth->index = index;
@@ -524,6 +524,7 @@ static struct clk * __init ti_fapll_synth_setup(struct fapll_data *fd,
free:
kfree(synth);
+free_init:
kfree(init);
return clk;
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 14:20:18 +0100
The kfree() function was called in one case by
the of_mux_clk_setup() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus return directly after a call of the function “kzalloc” failed
at the beginning.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/mux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/ti/mux.c b/drivers/clk/ti/mux.c
index 1ebafa386be6..ab1205fa40d6 100644
--- a/drivers/clk/ti/mux.c
+++ b/drivers/clk/ti/mux.c
@@ -182,7 +182,7 @@ static void of_mux_clk_setup(struct device_node *node)
}
parent_names = kzalloc((sizeof(char *) * num_parents), GFP_KERNEL);
if (!parent_names)
- goto cleanup;
+ return;
of_clk_parent_fill(node, parent_names, num_parents);
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 15:46:04 +0100
The kfree() function was called in a few cases by
the _ti_omap4_clkctrl_setup() function during error handling
even if the passed variable contained a null pointer.
Thus adjust jump targets.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/clkctrl.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 607e34d8e289..82b48548818b 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -681,11 +681,11 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
reg_data->offset, 0,
legacy_naming);
if (!init.name)
- goto cleanup;
+ goto free_clkctrl_name;
clkctrl_clk = kzalloc(sizeof(*clkctrl_clk), GFP_KERNEL);
if (!clkctrl_clk)
- goto cleanup;
+ goto free_init_name;
init.ops = &omap4_clkctrl_clk_ops;
hw->hw.init = &init;
@@ -711,10 +711,12 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
return;
cleanup:
- kfree(hw);
+ kfree(clkctrl_clk);
+free_init_name:
kfree(init.name);
+free_clkctrl_name:
kfree(clkctrl_name);
- kfree(clkctrl_clk);
+ kfree(hw);
}
CLK_OF_DECLARE(ti_omap4_clkctrl_clock, "ti,clkctrl",
_ti_omap4_clkctrl_setup);
--
2.43.0
From: Markus Elfring <[email protected]>
Date: Sun, 24 Dec 2023 15:56:08 +0100
Add a jump target so that a bit of exception handling can be better reused
at the end of this function.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/clk/ti/clkctrl.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/ti/clkctrl.c b/drivers/clk/ti/clkctrl.c
index 82b48548818b..5a1bd176160c 100644
--- a/drivers/clk/ti/clkctrl.c
+++ b/drivers/clk/ti/clkctrl.c
@@ -590,10 +590,9 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
if (clkctrl_name) {
provider->clkdm_name = kasprintf(GFP_KERNEL,
"%s_clkdm", clkctrl_name);
- if (!provider->clkdm_name) {
- kfree(provider);
- return;
- }
+ if (!provider->clkdm_name)
+ goto free_provider;
+
goto clkdm_found;
}
@@ -603,10 +602,8 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
*/
if (legacy_naming) {
provider->clkdm_name = kasprintf(GFP_KERNEL, "%pOFnxxx", node->parent);
- if (!provider->clkdm_name) {
- kfree(provider);
- return;
- }
+ if (!provider->clkdm_name)
+ goto free_provider;
/*
* Create default clkdm name, replace _cm from end of parent
@@ -615,10 +612,8 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
provider->clkdm_name[strlen(provider->clkdm_name) - 2] = 0;
} else {
provider->clkdm_name = kasprintf(GFP_KERNEL, "%pOFn", node);
- if (!provider->clkdm_name) {
- kfree(provider);
- return;
- }
+ if (!provider->clkdm_name)
+ goto free_provider;
/*
* Create default clkdm name, replace _clkctrl from end of
@@ -710,4 +705,8 @@ static void __init _ti_omap4_clkctrl_setup(struct device_node *node)
return;
+free_provider:
+ kfree(provider);
+ return;
+
cleanup:
--
2.43.0
On Sun, Dec 24, 2023 at 05:36:57PM +0100, Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Sun, 24 Dec 2023 11:15:27 +0100
>
> The kfree() function was called in up to three cases by
> the of_omap2_apll_setup() function during error handling
> even if the passed variable contained a null pointer.
> This issue was detected by using the Coccinelle software.
> * Split a condition check.
>
> * Adjust jump targets.
>
> * Delete three initialisations which became unnecessary
> with this refactoring.
Instead, make use of cleanup.h.
--
With Best Regards,
Andy Shevchenko
On Sun, Dec 24, 2023 at 05:33:53PM +0100, Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Sun, 24 Dec 2023 17:03:21 +0100
>
> Several update suggestions were taken into account
> from static source code analysis.
Unneeded churn, if you want to make it better, switch the code to use
cleanup.h.
--
With Best Regards,
Andy Shevchenko
* Andy Shevchenko <[email protected]> [231227 16:39]:
> On Sun, Dec 24, 2023 at 05:33:53PM +0100, Markus Elfring wrote:
> > From: Markus Elfring <[email protected]>
> > Date: Sun, 24 Dec 2023 17:03:21 +0100
> >
> > Several update suggestions were taken into account
> > from static source code analysis.
>
> Unneeded churn, if you want to make it better, switch the code to use
> cleanup.h.
Yes cleanup.h sounds good to me too.
Regards,
Tony