[PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

[PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

[Himangi Saraogi]

nhi->rx_rings does not have type as struct tb_ring *, as it is a double
pointer. So, the elements of the array should have pointer type, not
structure type. The advantage of kcalloc is, that will prevent integer
overflows which could result from the multiplication of number of
elements and size and it is also a bit nicer to read.

The Coccinelle semantic patch that makes the first change is as follows:

// <smpl>
@disable sizeof_type_expr@
type T;
T **x;
@@

  x =
  <+...sizeof(
- T
+ *x
  )...+>
// </smpl>

Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
v2: Use kcalloc
v3: Add maintainer in To
 drivers/thunderbolt/nhi.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 2054fbf..c68fe12 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -569,12 +569,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 			 nhi->hop_count);
 	INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
 
-	nhi->tx_rings = devm_kzalloc(&pdev->dev,
-				     nhi->hop_count * sizeof(struct tb_ring),
-				     GFP_KERNEL);
-	nhi->rx_rings = devm_kzalloc(&pdev->dev,
-				     nhi->hop_count * sizeof(struct tb_ring),
-				     GFP_KERNEL);
+	nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
+				     sizeof(*nhi->tx_rings), GFP_KERNEL);
+	nhi->rx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
+				     sizeof(*nhi->rx_rings), GFP_KERNEL);
 	if (!nhi->tx_rings || !nhi->rx_rings)
 		return -ENOMEM;
 
-- 
1.9.1
	

Re: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

[Andreas Noever]

On Wed, Jul 9, 2014 at 12:31 AM, Himangi Saraogi <himangi774@gmail.com> wrote:
> nhi->rx_rings does not have type as struct tb_ring *, as it is a double
> pointer. So, the elements of the array should have pointer type, not
> structure type. The advantage of kcalloc is, that will prevent integer
> overflows which could result from the multiplication of number of
> elements and size and it is also a bit nicer to read.
>
> The Coccinelle semantic patch that makes the first change is as follows:
>
> // <smpl>
> @disable sizeof_type_expr@
> type T;
> T **x;
> @@
>
>   x =
>   <+...sizeof(
> - T
> + *x
>   )...+>
> // </smpl>
>
> Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
> v2: Use kcalloc
> v3: Add maintainer in To
>  drivers/thunderbolt/nhi.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index 2054fbf..c68fe12 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -569,12 +569,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>                          nhi->hop_count);
>         INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
>
> -       nhi->tx_rings = devm_kzalloc(&pdev->dev,
> -                                    nhi->hop_count * sizeof(struct tb_ring),
> -                                    GFP_KERNEL);
> -       nhi->rx_rings = devm_kzalloc(&pdev->dev,
> -                                    nhi->hop_count * sizeof(struct tb_ring),
> -                                    GFP_KERNEL);
> +       nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
> +                                    sizeof(*nhi->tx_rings), GFP_KERNEL);
> +       nhi->rx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
> +                                    sizeof(*nhi->rx_rings), GFP_KERNEL);
>         if (!nhi->tx_rings || !nhi->rx_rings)
>                 return -ENOMEM;
>
> --
> 1.9.1
>

Thanks for catching this.
Acked-by: Andreas Noever <andreas.noever@gmail.com>

Re: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

[Andreas Noever]

Greg, can you add this to your char-misc tree (instead of the kzalloc
patch, or do you need a rebased patch)?

Thanks,
Andreas

On Thu, Jul 10, 2014 at 6:45 PM, Andreas Noever
<andreas.noever@gmail.com> wrote:
> On Wed, Jul 9, 2014 at 12:31 AM, Himangi Saraogi <himangi774@gmail.com> wrote:
>> nhi->rx_rings does not have type as struct tb_ring *, as it is a double
>> pointer. So, the elements of the array should have pointer type, not
>> structure type. The advantage of kcalloc is, that will prevent integer
>> overflows which could result from the multiplication of number of
>> elements and size and it is also a bit nicer to read.
>>
>> The Coccinelle semantic patch that makes the first change is as follows:
>>
>> // <smpl>
>> @disable sizeof_type_expr@
>> type T;
>> T **x;
>> @@
>>
>>   x =
>>   <+...sizeof(
>> - T
>> + *x
>>   )...+>
>> // </smpl>
>>
>> Signed-off-by: Himangi Saraogi <himangi774@gmail.com>
>> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>> ---
>> v2: Use kcalloc
>> v3: Add maintainer in To
>>  drivers/thunderbolt/nhi.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index 2054fbf..c68fe12 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -569,12 +569,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>                          nhi->hop_count);
>>         INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
>>
>> -       nhi->tx_rings = devm_kzalloc(&pdev->dev,
>> -                                    nhi->hop_count * sizeof(struct tb_ring),
>> -                                    GFP_KERNEL);
>> -       nhi->rx_rings = devm_kzalloc(&pdev->dev,
>> -                                    nhi->hop_count * sizeof(struct tb_ring),
>> -                                    GFP_KERNEL);
>> +       nhi->tx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
>> +                                    sizeof(*nhi->tx_rings), GFP_KERNEL);
>> +       nhi->rx_rings = devm_kcalloc(&pdev->dev, nhi->hop_count,
>> +                                    sizeof(*nhi->rx_rings), GFP_KERNEL);
>>         if (!nhi->tx_rings || !nhi->rx_rings)
>>                 return -ENOMEM;
>>
>> --
>> 1.9.1
>>
>
> Thanks for catching this.
> Acked-by: Andreas Noever <andreas.noever@gmail.com>

Re: [PATCH v3] thunderbolt: Use kcalloc and correct the argument to sizeof

[Greg Kroah-Hartman]

On Fri, Jul 11, 2014 at 07:14:23PM +0200, Andreas Noever wrote:
> Greg, can you add this to your char-misc tree (instead of the kzalloc
> patch, or do you need a rebased patch)?

I need a patch on top of my char-misc tree as the kzalloc version is
already applied there.

thanks,

greg k-h