* [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
@ 2014-07-25 7:09 pramod.gurav.etc
2014-07-25 8:29 ` Sachin Kamat
0 siblings, 1 reply; 4+ messages in thread
From: pramod.gurav.etc @ 2014-07-25 7:09 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Pramod Gurav, Dmitry Torokhov, Lejun Zhu, Sachin Kamat
From: Pramod Gurav <pramod.gurav@smartplayin.com>
This patch does below:
- Removes kfree done on data allocated with devm_zalloc in probe
path of the driver.
- Adds a check on return value from devm_kzalloc which was missing
and renames a lable from err_free_mem to err_mem.
- Adds couple of dev_err on failure to allocate memory
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Lejun Zhu <lejun.zhu@linux.intel.com>
CC: Sachin Kamat <sachin.kamat@linaro.org>
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5a6334b..ac82971 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
sizeof(*gpio_keys_pdata) +
sizeof(*gpio_keys) * MAX_NBUTTONS,
GFP_KERNEL);
+ if (!gpio_keys_pdata) {
+ dev_err(&pdev->dev,
+ "Can't allocate memory for gpio_keys_platform_data\n");
+ error = -ENOMEM;
+ goto err_mem;
+ }
+
gpio_keys = (void *)(gpio_keys_pdata + 1);
for (info = button_info; info->name; info++) {
@@ -104,7 +111,7 @@ soc_button_device_create(struct pnp_dev *pdev,
if (n_buttons == 0) {
error = -ENODEV;
- goto err_free_mem;
+ goto err_mem;
}
gpio_keys_pdata->buttons = gpio_keys;
@@ -114,7 +121,7 @@ soc_button_device_create(struct pnp_dev *pdev,
pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
if (!pd) {
error = -ENOMEM;
- goto err_free_mem;
+ goto err_mem;
}
error = platform_device_add_data(pd, gpio_keys_pdata,
@@ -130,8 +137,7 @@ soc_button_device_create(struct pnp_dev *pdev,
err_free_pdev:
platform_device_put(pd);
-err_free_mem:
- devm_kfree(&pdev->dev, gpio_keys_pdata);
+err_mem:
return ERR_PTR(error);
}
@@ -155,8 +161,11 @@ static int soc_button_pnp_probe(struct pnp_dev *pdev,
int error;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
+ if (!priv) {
+ dev_err(&pdev->dev,
+ "Can't allocate memory for soc_button_data\n");
return -ENOMEM;
+ }
pnp_set_drvdata(pdev, priv);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
2014-07-25 7:09 [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc pramod.gurav.etc
@ 2014-07-25 8:29 ` Sachin Kamat
2014-07-25 8:48 ` pramod gurav
2014-07-25 9:02 ` David Rientjes
0 siblings, 2 replies; 4+ messages in thread
From: Sachin Kamat @ 2014-07-25 8:29 UTC (permalink / raw)
To: pramod.gurav.etc
Cc: linux-input, open list, Pramod Gurav, Dmitry Torokhov, Lejun Zhu,
Sachin Kamat
On Fri, Jul 25, 2014 at 12:39 PM, <pramod.gurav.etc@gmail.com> wrote:
> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>
> This patch does below:
> - Removes kfree done on data allocated with devm_zalloc in probe
> path of the driver.
> - Adds a check on return value from devm_kzalloc which was missing
> and renames a lable from err_free_mem to err_mem.
> - Adds couple of dev_err on failure to allocate memory
>
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
> CC: Sachin Kamat <sachin.kamat@linaro.org>
>
> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> ---
> drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 5a6334b..ac82971 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
> sizeof(*gpio_keys_pdata) +
> sizeof(*gpio_keys) * MAX_NBUTTONS,
> GFP_KERNEL);
> + if (!gpio_keys_pdata) {
> + dev_err(&pdev->dev,
> + "Can't allocate memory for gpio_keys_platform_data\n");
OOM message is not needed as memory subsystem will take care of it.
> + error = -ENOMEM;
> + goto err_mem;
Return directly from here.
> + }
> +
> gpio_keys = (void *)(gpio_keys_pdata + 1);
>
> for (info = button_info; info->name; info++) {
> @@ -104,7 +111,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>
> if (n_buttons == 0) {
> error = -ENODEV;
> - goto err_free_mem;
> + goto err_mem;
ditto
> }
>
> gpio_keys_pdata->buttons = gpio_keys;
> @@ -114,7 +121,7 @@ soc_button_device_create(struct pnp_dev *pdev,
> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
> if (!pd) {
> error = -ENOMEM;
> - goto err_free_mem;
> + goto err_mem;
ditto
> }
>
> error = platform_device_add_data(pd, gpio_keys_pdata,
> @@ -130,8 +137,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>
> err_free_pdev:
> platform_device_put(pd);
> -err_free_mem:
> - devm_kfree(&pdev->dev, gpio_keys_pdata);
> +err_mem:
This label would not be required once returned directly from above.
> return ERR_PTR(error);
> }
>
> @@ -155,8 +161,11 @@ static int soc_button_pnp_probe(struct pnp_dev *pdev,
> int error;
>
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> + if (!priv) {
> + dev_err(&pdev->dev,
> + "Can't allocate memory for soc_button_data\n");
OOM message is not needed as memory subsystem will take care of it.
--
Regards,
Sachin.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
2014-07-25 8:29 ` Sachin Kamat
@ 2014-07-25 8:48 ` pramod gurav
2014-07-25 9:02 ` David Rientjes
1 sibling, 0 replies; 4+ messages in thread
From: pramod gurav @ 2014-07-25 8:48 UTC (permalink / raw)
To: Sachin Kamat
Cc: linux-input, open list, Pramod Gurav, Dmitry Torokhov, Lejun Zhu,
Sachin Kamat
Thanks Sachin for the review. Will resend the patch addressing your
review comments.
On Fri, Jul 25, 2014 at 1:59 PM, Sachin Kamat <spk.linux@gmail.com> wrote:
> On Fri, Jul 25, 2014 at 12:39 PM, <pramod.gurav.etc@gmail.com> wrote:
>> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>>
>> This patch does below:
>> - Removes kfree done on data allocated with devm_zalloc in probe
>> path of the driver.
>> - Adds a check on return value from devm_kzalloc which was missing
>> and renames a lable from err_free_mem to err_mem.
>> - Adds couple of dev_err on failure to allocate memory
>>
>> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
>> CC: Sachin Kamat <sachin.kamat@linaro.org>
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>> ---
>> drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
>> index 5a6334b..ac82971 100644
>> --- a/drivers/input/misc/soc_button_array.c
>> +++ b/drivers/input/misc/soc_button_array.c
>> @@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
>> sizeof(*gpio_keys_pdata) +
>> sizeof(*gpio_keys) * MAX_NBUTTONS,
>> GFP_KERNEL);
>> + if (!gpio_keys_pdata) {
>> + dev_err(&pdev->dev,
>> + "Can't allocate memory for gpio_keys_platform_data\n");
>
> OOM message is not needed as memory subsystem will take care of it.
>
>> + error = -ENOMEM;
>> + goto err_mem;
>
> Return directly from here.
>
>> + }
>> +
>> gpio_keys = (void *)(gpio_keys_pdata + 1);
>>
>> for (info = button_info; info->name; info++) {
>> @@ -104,7 +111,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>>
>> if (n_buttons == 0) {
>> error = -ENODEV;
>> - goto err_free_mem;
>> + goto err_mem;
>
> ditto
>
>> }
>>
>> gpio_keys_pdata->buttons = gpio_keys;
>> @@ -114,7 +121,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>> pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
>> if (!pd) {
>> error = -ENOMEM;
>> - goto err_free_mem;
>> + goto err_mem;
>
> ditto
>
>> }
>>
>> error = platform_device_add_data(pd, gpio_keys_pdata,
>> @@ -130,8 +137,7 @@ soc_button_device_create(struct pnp_dev *pdev,
>>
>> err_free_pdev:
>> platform_device_put(pd);
>> -err_free_mem:
>> - devm_kfree(&pdev->dev, gpio_keys_pdata);
>> +err_mem:
>
> This label would not be required once returned directly from above.
>
>> return ERR_PTR(error);
>> }
>>
>> @@ -155,8 +161,11 @@ static int soc_button_pnp_probe(struct pnp_dev *pdev,
>> int error;
>>
>> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> - if (!priv)
>> + if (!priv) {
>> + dev_err(&pdev->dev,
>> + "Can't allocate memory for soc_button_data\n");
>
> OOM message is not needed as memory subsystem will take care of it.
>
>
> --
> Regards,
> Sachin.
--
Thanks and Regards
Pramod
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
2014-07-25 8:29 ` Sachin Kamat
2014-07-25 8:48 ` pramod gurav
@ 2014-07-25 9:02 ` David Rientjes
1 sibling, 0 replies; 4+ messages in thread
From: David Rientjes @ 2014-07-25 9:02 UTC (permalink / raw)
To: Sachin Kamat
Cc: pramod.gurav.etc, linux-input, open list, Pramod Gurav,
Dmitry Torokhov, Lejun Zhu, Sachin Kamat
On Fri, 25 Jul 2014, Sachin Kamat wrote:
> On Fri, Jul 25, 2014 at 12:39 PM, <pramod.gurav.etc@gmail.com> wrote:
> > From: Pramod Gurav <pramod.gurav@smartplayin.com>
> >
> > This patch does below:
> > - Removes kfree done on data allocated with devm_zalloc in probe
> > path of the driver.
> > - Adds a check on return value from devm_kzalloc which was missing
> > and renames a lable from err_free_mem to err_mem.
> > - Adds couple of dev_err on failure to allocate memory
> >
> > CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > CC: Lejun Zhu <lejun.zhu@linux.intel.com>
> > CC: Sachin Kamat <sachin.kamat@linaro.org>
> >
> > Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
> > ---
> > drivers/input/misc/soc_button_array.c | 19 ++++++++++++++-----
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> > index 5a6334b..ac82971 100644
> > --- a/drivers/input/misc/soc_button_array.c
> > +++ b/drivers/input/misc/soc_button_array.c
> > @@ -83,6 +83,13 @@ soc_button_device_create(struct pnp_dev *pdev,
> > sizeof(*gpio_keys_pdata) +
> > sizeof(*gpio_keys) * MAX_NBUTTONS,
> > GFP_KERNEL);
> > + if (!gpio_keys_pdata) {
> > + dev_err(&pdev->dev,
> > + "Can't allocate memory for gpio_keys_platform_data\n");
>
> OOM message is not needed as memory subsystem will take care of it.
>
In the worst case, the memory subsystem will print that a process was
killed for this allocation to succeed so it's never actually going to
fail. However, it is indeed frowed upon to print your own oom message so
you're right that it should be removed.
Same comment about soc_button_pnp_probe().
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-25 9:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-25 7:09 [PATCH] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc pramod.gurav.etc
2014-07-25 8:29 ` Sachin Kamat
2014-07-25 8:48 ` pramod gurav
2014-07-25 9:02 ` David Rientjes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).