2013-05-15 10:54:06

by Leon Romanovsky

Subject: [PATCH] drivers: mfd: sec-code: Fix sizeof argument

Signed-off-by: Leon Romanovsky <[email protected]>
---
drivers/mfd/sec-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
index 77ee26ef..5b740a3 100644
--- a/drivers/mfd/sec-core.c
+++ b/drivers/mfd/sec-core.c
@@ -121,7 +121,7 @@ static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
{
struct sec_platform_data *pd;

- pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+ pd = devm_kzalloc(dev, sizeof(struct sec_platform_data), GFP_KERNEL);
if (!pd) {
dev_err(dev, "could not allocate memory for pdata\n");
return ERR_PTR(-ENOMEM);
--
1.8.1.5

2013-05-16 22:13:25

by Samuel Ortiz

Subject: Re: [PATCH] drivers: mfd: sec-code: Fix sizeof argument

Hi Leon,

On Wed, May 15, 2013 at 01:53:56PM +0300, Leon Romanovsky wrote:
> Signed-off-by: Leon Romanovsky <[email protected]>
> ---
> drivers/mfd/sec-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> index 77ee26ef..5b740a3 100644
> --- a/drivers/mfd/sec-core.c
> +++ b/drivers/mfd/sec-core.c
> @@ -121,7 +121,7 @@ static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
> {
> struct sec_platform_data *pd;
>
> - pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> + pd = devm_kzalloc(dev, sizeof(struct sec_platform_data), GFP_KERNEL);
How is that fixing anything ?

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/

2013-05-17 17:50:58

by Leon Romanovsky

Subject: Re: [PATCH] drivers: mfd: sec-code: Fix sizeof argument

On Fri, May 17, 2013 at 1:13 AM, Samuel Ortiz <[email protected]> wrote:
> Hi Leon,
>
> On Wed, May 15, 2013 at 01:53:56PM +0300, Leon Romanovsky wrote:
>> Signed-off-by: Leon Romanovsky <[email protected]>
>> ---
>> drivers/mfd/sec-core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
>> index 77ee26ef..5b740a3 100644
>> --- a/drivers/mfd/sec-core.c
>> +++ b/drivers/mfd/sec-core.c
>> @@ -121,7 +121,7 @@ static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
>> {
>> struct sec_platform_data *pd;
>>
>> - pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
>> + pd = devm_kzalloc(dev, sizeof(struct sec_platform_data), GFP_KERNEL);
> How is that fixing anything ?
Technically you are right, this fix brings code to be align to common
code convention and allows to automatic tools correctly parse it.

>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/



--
Leon Romanovsky | Independent Linux Consultant
http://www.leon.nu | [email protected]

2013-05-17 17:58:21

by Joe Perches

Subject: Re: [PATCH] drivers: mfd: sec-code: Fix sizeof argument

On Fri, 2013-05-17 at 20:50 +0300, Leon Romanovsky wrote:
> On Fri, May 17, 2013 at 1:13 AM, Samuel Ortiz <[email protected]> wrote:
> > On Wed, May 15, 2013 at 01:53:56PM +0300, Leon Romanovsky wrote:
> >> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
[]
> >> @@ -121,7 +121,7 @@ static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
[]
> >> - pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> >> + pd = devm_kzalloc(dev, sizeof(struct sec_platform_data), GFP_KERNEL);
> > How is that fixing anything ?
> Technically you are right, this fix brings code to be align to common
> code convention and allows to automatic tools correctly parse it.

Not really.
Common coding convention is actually the original code.

from: Documentation/CodingStyle:

Chapter 14: Allocating memory
[]
The preferred form for passing a size of a struct is the following:

p = kmalloc(sizeof(*p), ...);

2013-05-17 18:17:18

by Leon Romanovsky

Subject: Re: [PATCH] drivers: mfd: sec-code: Fix sizeof argument

On Fri, May 17, 2013 at 8:58 PM, Joe Perches <[email protected]> wrote:
> On Fri, 2013-05-17 at 20:50 +0300, Leon Romanovsky wrote:
>> On Fri, May 17, 2013 at 1:13 AM, Samuel Ortiz <[email protected]> wrote:
>> > On Wed, May 15, 2013 at 01:53:56PM +0300, Leon Romanovsky wrote:
>> >> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c
> []
>> >> @@ -121,7 +121,7 @@ static struct sec_platform_data *sec_pmic_i2c_parse_dt_pdata(
> []
>> >> - pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
>> >> + pd = devm_kzalloc(dev, sizeof(struct sec_platform_data), GFP_KERNEL);
>> > How is that fixing anything ?
>> Technically you are right, this fix brings code to be align to common
>> code convention and allows to automatic tools correctly parse it.
>
> Not really.
> Common coding convention is actually the original code.
>
> from: Documentation/CodingStyle:
>
> Chapter 14: Allocating memory
> []
> The preferred form for passing a size of a struct is the following:
>
> p = kmalloc(sizeof(*p), ...);
>
>
Thanks Joe, it is good to know, but from my simple grep, most
developers doesn't follow with this guide:
leon@tux ~/dev/kernel/linux-staging $ grep -r "sizeof(struct" * | wc -l
25865
leon@tux ~/dev/kernel/linux-staging $ grep -r "sizeof(\*" * | wc -l
14568

--
Leon Romanovsky | Independent Linux Consultant
http://www.leon.nu | [email protected]