On Wed, Feb 12, 2014 at 12:13:00AM +0100, Christopher Heiny wrote:
> Correctly free driver related data when initialization fails.
> 
> Trivial: Clarify a diagnostic message.
> 
> Signed-off-by: Christopher Heiny <[email protected]>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Benjamin Tissoires <[email protected]>
> Cc: Linux Walleij <[email protected]>
> Cc: David Herrmann <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> 
> ---
> 
>  drivers/input/rmi4/rmi_f01.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 381ad60..e4a6df9 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -149,7 +149,7 @@ static int rmi_f01_alloc_memory(struct rmi_function *fn,
>  
>       f01 = devm_kzalloc(&fn->dev, sizeof(struct f01_data), GFP_KERNEL);
>       if (!f01) {
> -             dev_err(&fn->dev, "Failed to allocate fn_01_data.\n");
> +             dev_err(&fn->dev, "Failed to allocate f01_data.\n");

Just remove this printout, as it won't help any user in the case of OOM.
Additionally, there's already plenty of (more useful) information
printed out if kmalloc fails.

>               return -ENOMEM;
>       }
>  
> @@ -158,6 +158,7 @@ static int rmi_f01_alloc_memory(struct rmi_function *fn,
>                       GFP_KERNEL);
>       if (!f01->device_control.interrupt_enable) {
>               dev_err(&fn->dev, "Failed to allocate interrupt enable.\n");
> +             devm_kfree(&fn->dev, f01);

Unnecessary, devres_release_all() will release this, from:
        - really_probe() -> rmi_function_probe() -> rmi_f01_probe()
        - driver_probe_device()
        - __driver_attach()
        - driver_attach()
        - bus_add_driver()
        - driver_register()
        - __rmi_register_function_handler()

>               return -ENOMEM;
>       }
>       fn->data = f01;
> @@ -381,7 +382,8 @@ static int rmi_f01_initialize(struct rmi_function *fn)
>       return 0;
>  
>   error_exit:
> -     kfree(data);
> +     devm_kfree(&fn->dev, data->device_control.interrupt_enable);
> +     devm_kfree(&fn->dev, data);

Same as above for these two.

>       return error;
>  }
>  

Generally devm_ release functions are unnecessary to call, as all
resources will get released on driver detach, whether abnormal or not.

-Courtney
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to