On Thu, 22 Sep 2016, Arvind Yadav wrote:

Darn. Take your time and think about stuff instead of sending a new patch
out 5 seconds after you got a review comment.

> Subject : [v2] clocksrouce/timer-imz-gpt: Prevent resource leaks in error path

1) This lacks a [PATCH v2] prefix

2) clocksrouce is not a valid subsytem

> Free memory and memory mapping, if mxc_timer_init_dt is not successful.

3) There is no memory mapping to free. Simply because that code maps a
   peripheral and not memory.

Stop acting like a shell script or your mail address ends up in one which
sends your mails immediately to /dev/null.

> diff --git a/drivers/clocksource/timer-imx-gpt.c 
> b/drivers/clocksource/timer-imx-gpt.c
> index f595460..19f6860 100644
> --- a/drivers/clocksource/timer-imx-gpt.c
> +++ b/drivers/clocksource/timer-imx-gpt.c
> @@ -489,12 +489,17 @@ static int __init mxc_timer_init_dt(struct device_node 
> *np,  enum imx_gpt_type t
>               return -ENOMEM;
>  
>       imxtm->base = of_iomap(np, 0);
> -     if (!imxtm->base)
> +     if (!imxtm->base) {
> +             kfree(imxtm);
>               return -ENXIO;
> +     }
>  
>       imxtm->irq = irq_of_parse_and_map(np, 0);
> -     if (imxtm->irq <= 0)
> +     if (imxtm->irq <= 0) {
> +             iounmap(imxtm->base);
> +             kfree(imxtm);
>               return -EINVAL;
> +     }
>  
>       imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
>  
> @@ -506,8 +511,11 @@ static int __init mxc_timer_init_dt(struct device_node 
> *np,  enum imx_gpt_type t
>       imxtm->type = type;
>  
>       ret = _mxc_timer_init(imxtm);
> -     if (ret)
> +     if (ret) {
> +             iounmap(imxtm->base);
> +             kfree(imxtm);

Ever heard about the concept of goto? Just adding blindly iounmap and kfree
at every place plugs the leak, but is far away from proper kernel code.

Thanks,

        tglx

Reply via email to