2014-07-06 16:13:50

by Himangi Saraogi

[permalink] [raw]
Subject: [PATCH] thunderbolt: Correct the size argument to devm_kzalloc

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 Coccinelle semantic patch that makes this change is as follows:

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

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

Signed-off-by: Himangi Saraogi <[email protected]>
Acked-by: Julia Lawall <[email protected]>
---
drivers/thunderbolt/nhi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 2054fbf..ce72f31 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -570,10 +570,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);

nhi->tx_rings = devm_kzalloc(&pdev->dev,
- nhi->hop_count * sizeof(struct tb_ring),
+ nhi->hop_count * sizeof(*nhi->tx_rings),
GFP_KERNEL);
nhi->rx_rings = devm_kzalloc(&pdev->dev,
- nhi->hop_count * sizeof(struct tb_ring),
+ nhi->hop_count * sizeof(*nhi->rx_rings),
GFP_KERNEL);
if (!nhi->tx_rings || !nhi->rx_rings)
return -ENOMEM;
--
1.9.1


2014-07-06 16:31:08

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Correct the size argument to devm_kzalloc

On Sun, 2014-07-06 at 21:43 +0530, Himangi Saraogi 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.
[]
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
[]
> @@ -570,10 +570,10 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> INIT_WORK(&nhi->interrupt_work, nhi_interrupt_work);
>
> nhi->tx_rings = devm_kzalloc(&pdev->dev,
> - nhi->hop_count * sizeof(struct tb_ring),
> + nhi->hop_count * sizeof(*nhi->tx_rings),
> GFP_KERNEL);
> nhi->rx_rings = devm_kzalloc(&pdev->dev,
> - nhi->hop_count * sizeof(struct tb_ring),
> + nhi->hop_count * sizeof(*nhi->rx_rings),
> GFP_KERNEL);

It could use devm_kcalloc here too.

btw: are there many false positives with the cocci test?

2014-07-06 23:58:37

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Correct the size argument to devm_kzalloc

On Sun, Jul 06, 2014 at 09:43:42PM +0530, Himangi Saraogi 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 Coccinelle semantic patch that makes this change is as follows:
>
> // <smpl>
> @disable sizeof_type_expr@
> type T;
> T **x;
> @@
>
> x =
> <+...sizeof(
> - T
> + *x
> )...+>
> // </smpl>
>
> Signed-off-by: Himangi Saraogi <[email protected]>
> Acked-by: Julia Lawall <[email protected]>
> ---
> drivers/thunderbolt/nhi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Any reason you didn't cc: the developers in charge of this code so they
see it and can pick it up? Otherwise it's just going to float around on
the mailing list, listless and sad.

greg k-h

2014-07-07 04:32:25

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Correct the size argument to devm_kzalloc



On Sun, 6 Jul 2014, Greg KH wrote:

> On Sun, Jul 06, 2014 at 09:43:42PM +0530, Himangi Saraogi 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 Coccinelle semantic patch that makes this change is as follows:
> >
> > // <smpl>
> > @disable sizeof_type_expr@
> > type T;
> > T **x;
> > @@
> >
> > x =
> > <+...sizeof(
> > - T
> > + *x
> > )...+>
> > // </smpl>
> >
> > Signed-off-by: Himangi Saraogi <[email protected]>
> > Acked-by: Julia Lawall <[email protected]>
> > ---
> > drivers/thunderbolt/nhi.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Any reason you didn't cc: the developers in charge of this code so they
> see it and can pick it up? Otherwise it's just going to float around on
> the mailing list, listless and sad.

They don't seem to be in the MAINTAINERS file.

Himangi, you would see some people if you drop the --nogit-fallback
argument from get_maintainers. Maybe that is too strict.

julia

2014-07-07 05:42:27

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] thunderbolt: Correct the size argument to devm_kzalloc

On Mon, Jul 07, 2014 at 06:32:04AM +0200, Julia Lawall wrote:
>
>
> On Sun, 6 Jul 2014, Greg KH wrote:
>
> > On Sun, Jul 06, 2014 at 09:43:42PM +0530, Himangi Saraogi 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 Coccinelle semantic patch that makes this change is as follows:
> > >
> > > // <smpl>
> > > @disable sizeof_type_expr@
> > > type T;
> > > T **x;
> > > @@
> > >
> > > x =
> > > <+...sizeof(
> > > - T
> > > + *x
> > > )...+>
> > > // </smpl>
> > >
> > > Signed-off-by: Himangi Saraogi <[email protected]>
> > > Acked-by: Julia Lawall <[email protected]>
> > > ---
> > > drivers/thunderbolt/nhi.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Any reason you didn't cc: the developers in charge of this code so they
> > see it and can pick it up? Otherwise it's just going to float around on
> > the mailing list, listless and sad.
>
> They don't seem to be in the MAINTAINERS file.

Ah, good point. Andreas, care to send me a patch adding yourself to the
MAINTAINERS file for drivers/thunderbolt/* so that you will get patches
like this?

thanks,

greg k-h