Re: [PATCH 3/4] clk/Renesas-MSTP: Less function calls in cpg_mstp_clocks_init() after error detection

[Geert Uytterhoeven]

Hi Markus,

On Wed, Sep 14, 2016 at 10:03 PM, SF Markus Elfring
<elfring@xxxxxxxxxxxxxxxxxxxxx> wrote:
> From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> Date: Wed, 14 Sep 2016 21:30:27 +0200
>
> The kfree() function was called in up to two cases
> by the cpg_mstp_clocks_init() function during error handling even if
> the passed variable contained a null pointer.

It's perfectly legal to call kfree() on a NULL pointer.

> * Split a condition check for memory allocation failures so that
>   each pointer from these function calls will be checked immediately.
>
>   See also background information:
>   Topic "CWE-754: Improper check for unusual or exceptional conditions"
>   Link: https://cwe.mitre.org/data/definitions/754.html
>
> * Return directly after a call of the function "kzalloc" failed
>   at the beginning.

Both calls are already close together.

> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/clk/renesas/clk-mstp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

In addition, your patch increases the LoC, IMHO without improving the code.

>
> diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c
> index 1fdc44b..6c82e0e 100644
> --- a/drivers/clk/renesas/clk-mstp.c
> +++ b/drivers/clk/renesas/clk-mstp.c
> @@ -167,10 +167,12 @@ static void __init cpg_mstp_clocks_init(struct device_node *np)
>         unsigned int i;
>
>         group = kzalloc(sizeof(*group), GFP_KERNEL);
> +       if (!group)
> +               return;
> +
>         clks = kmalloc_array(MSTP_MAX_CLOCKS, sizeof(*clks), GFP_KERNEL);
> -       if (group == NULL || clks == NULL) {
> +       if (!clks) {
>                 kfree(group);
> -               kfree(clks);
>                 return;
>         }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds