From: Julia Lawall <[email protected]>
Test the just-initialized value rather than some other one.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@
x = f(...);
(
if (\(x == NULL\|IS_ERR(x)\)) S
|
*if (\(y == NULL\|IS_ERR(y)\))
{ ... when != x
return ...; }
)
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/net/wireless/mwifiex/scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
index b28241c..d3111c9 100644
--- a/drivers/net/wireless/mwifiex/scan.c
+++ b/drivers/net/wireless/mwifiex/scan.c
@@ -1480,7 +1480,7 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
return -ENOMEM;
}
beacon_ie = kzalloc(ie_len, GFP_KERNEL);
- if (!bss_desc) {
+ if (!beacon_ie) {
dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
return -ENOMEM;
}
Hi Julia,
Thanks for the patch.
> -----Original Message-----
> From: Julia Lawall [mailto:[email protected]]
> Sent: Monday, August 22, 2011 7:16 AM
> To: Pierre Louis Aublin
> Cc: Bing Zhao; [email protected]; John W. Linville; [email protected];
> [email protected]; [email protected]
> Subject: [PATCH 4/4] drivers/net/wireless/mwifiex/scan.c: test the just-initialized value
>
> From: Julia Lawall <[email protected]>
>
> Test the just-initialized value rather than some other one.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r@
> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> statement S;
> @@
>
> x = f(...);
> (
> if (\(x == NULL\|IS_ERR(x)\)) S
> |
> *if (\(y == NULL\|IS_ERR(y)\))
> { ... when != x
> return ...; }
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <[email protected]>
Acked-by: Bing Zhao <[email protected]>
Thanks,
Bing
>
> ---
> drivers/net/wireless/mwifiex/scan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
> index b28241c..37ca2f9 100644
> --- a/drivers/net/wireless/mwifiex/scan.c
> +++ b/drivers/net/wireless/mwifiex/scan.c
> @@ -1480,8 +1480,8 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
> return -ENOMEM;
> }
> beacon_ie = kzalloc(ie_len, GFP_KERNEL);
> - if (!bss_desc) {
> - dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
> + if (!beacon_ie) {
> + dev_err(priv->adapter->dev, " failed to alloc beacon_ie\n");
> return -ENOMEM;
> }
> memcpy(beacon_ie, ie_buf, ie_len);
Am 22.08.2011 21:02, schrieb Bing Zhao:
> Hi Julia,
>
> Thanks for the patch.
>
>> -----Original Message-----
>> From: Julia Lawall [mailto:[email protected]]
>> Sent: Monday, August 22, 2011 7:16 AM
>> To: Pierre Louis Aublin
>> Cc: Bing Zhao; [email protected]; John W. Linville; [email protected];
>> [email protected]; [email protected]
>> Subject: [PATCH 4/4] drivers/net/wireless/mwifiex/scan.c: test the just-initialized value
>>
>> From: Julia Lawall <[email protected]>
>>
>> Test the just-initialized value rather than some other one.
>>
>> The semantic match that finds this problem is as follows:
>> (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @r@
>> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
>> statement S;
>> @@
>>
>> x = f(...);
>> (
>> if (\(x == NULL\|IS_ERR(x)\)) S
>> |
>> *if (\(y == NULL\|IS_ERR(y)\))
>> { ... when != x
>> return ...; }
>> )
>> // </smpl>
>>
>> Signed-off-by: Julia Lawall <[email protected]>
>
> Acked-by: Bing Zhao <[email protected]>
>
>
> Thanks,
> Bing
>
>>
>> ---
>> drivers/net/wireless/mwifiex/scan.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
>> index b28241c..37ca2f9 100644
>> --- a/drivers/net/wireless/mwifiex/scan.c
>> +++ b/drivers/net/wireless/mwifiex/scan.c
>> @@ -1480,8 +1480,8 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
>> return -ENOMEM;
>> }
>> beacon_ie = kzalloc(ie_len, GFP_KERNEL);
>> - if (!bss_desc) {
>> - dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
>> + if (!beacon_ie) {
>> + dev_err(priv->adapter->dev, " failed to alloc beacon_ie\n");
>> return -ENOMEM;
>> }
>> memcpy(beacon_ie, ie_buf, ie_len);
> --
this looks like a case for kmemdup()
just my 2 cents,
re,
wh
On 08/22/2011 09:00 AM, Julia Lawall wrote:
> From: Julia Lawall<[email protected]>
>
> Test the just-initialized value rather than some other one.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> //<smpl>
> @r@
> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> statement S;
> @@
>
> x = f(...);
> (
> if (\(x == NULL\|IS_ERR(x)\)) S
> |
> *if (\(y == NULL\|IS_ERR(y)\))
> { ... when != x
> return ...; }
> )
> //</smpl>
>
> Signed-off-by: Julia Lawall<[email protected]>
>
> ---
> drivers/net/wireless/mwifiex/scan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
> index b28241c..d3111c9 100644
> --- a/drivers/net/wireless/mwifiex/scan.c
> +++ b/drivers/net/wireless/mwifiex/scan.c
> @@ -1480,7 +1480,7 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
> return -ENOMEM;
> }
> beacon_ie = kzalloc(ie_len, GFP_KERNEL);
> - if (!bss_desc) {
> + if (!beacon_ie) {
> dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
> return -ENOMEM;
> }
The error message should also get the bss_desc => beacon_ie chang.
Larry
On Mon, 22 Aug 2011, Pierre Louis Aublin wrote:
> Hello all
>
> On 08/22/2011 04:00 PM, Julia Lawall wrote:
> > - if (!bss_desc) {
> > + if (!beacon_ie) {
> > dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
> Shouldn't we also modify the error message, from "failed to alloc bss_desc" to
> "failed to alloc beacon_ie" ?
Sure :)
julia
From: Julia Lawall <[email protected]>
Test the just-initialized value rather than some other one.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r@
identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
statement S;
@@
x = f(...);
(
if (\(x == NULL\|IS_ERR(x)\)) S
|
*if (\(y == NULL\|IS_ERR(y)\))
{ ... when != x
return ...; }
)
// </smpl>
Signed-off-by: Julia Lawall <[email protected]>
---
drivers/net/wireless/mwifiex/scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
index b28241c..37ca2f9 100644
--- a/drivers/net/wireless/mwifiex/scan.c
+++ b/drivers/net/wireless/mwifiex/scan.c
@@ -1480,8 +1480,8 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
return -ENOMEM;
}
beacon_ie = kzalloc(ie_len, GFP_KERNEL);
- if (!bss_desc) {
- dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
+ if (!beacon_ie) {
+ dev_err(priv->adapter->dev, " failed to alloc beacon_ie\n");
return -ENOMEM;
}
memcpy(beacon_ie, ie_buf, ie_len);
On Mon, 22 Aug 2011, Larry Finger wrote:
> On 08/22/2011 09:00 AM, Julia Lawall wrote:
> > From: Julia Lawall<[email protected]>
> >
> > Test the just-initialized value rather than some other one.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > //<smpl>
> > @r@
> > identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> > statement S;
> > @@
> >
> > x = f(...);
> > (
> > if (\(x == NULL\|IS_ERR(x)\)) S
> > |
> > *if (\(y == NULL\|IS_ERR(y)\))
> > { ... when != x
> > return ...; }
> > )
> > //</smpl>
> >
> > Signed-off-by: Julia Lawall<[email protected]>
> >
> > ---
> > drivers/net/wireless/mwifiex/scan.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/mwifiex/scan.c
> > b/drivers/net/wireless/mwifiex/scan.c
> > index b28241c..d3111c9 100644
> > --- a/drivers/net/wireless/mwifiex/scan.c
> > +++ b/drivers/net/wireless/mwifiex/scan.c
> > @@ -1480,7 +1480,7 @@ mwifiex_update_curr_bss_params(struct mwifiex_private
> > *priv,
> > return -ENOMEM;
> > }
> > beacon_ie = kzalloc(ie_len, GFP_KERNEL);
> > - if (!bss_desc) {
> > + if (!beacon_ie) {
> > dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
> > return -ENOMEM;
> > }
>
> The error message should also get the bss_desc => beacon_ie chang.
Thanks. Pierre Louis Aublin made the same observation and I resumitted
the patch.
julia
> Am 22.08.2011 21:02, schrieb Bing Zhao:
> > Hi Julia,
> >
> > Thanks for the patch.
> >
> >> -----Original Message-----
> >> From: Julia Lawall [mailto:[email protected]]
> >> Sent: Monday, August 22, 2011 7:16 AM
> >> To: Pierre Louis Aublin
> >> Cc: Bing Zhao; [email protected]; John W. Linville; [email protected];
> >> [email protected]; [email protected]
> >> Subject: [PATCH 4/4] drivers/net/wireless/mwifiex/scan.c: test the just-initialized value
> >>
> >> From: Julia Lawall <[email protected]>
> >>
> >> Test the just-initialized value rather than some other one.
> >>
> >> The semantic match that finds this problem is as follows:
> >> (http://coccinelle.lip6.fr/)
> >>
> >> // <smpl>
> >> @r@
> >> identifier x,y,f!={PTR_ERR,ERR_PTR,ERR_CAST};
> >> statement S;
> >> @@
> >>
> >> x = f(...);
> >> (
> >> if (\(x == NULL\|IS_ERR(x)\)) S
> >> |
> >> *if (\(y == NULL\|IS_ERR(y)\))
> >> { ... when != x
> >> return ...; }
> >> )
> >> // </smpl>
> >>
> >> Signed-off-by: Julia Lawall <[email protected]>
> >
> > Acked-by: Bing Zhao <[email protected]>
> >
> >
> > Thanks,
> > Bing
> >
> >>
> >> ---
> >> drivers/net/wireless/mwifiex/scan.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/mwifiex/scan.c b/drivers/net/wireless/mwifiex/scan.c
> >> index b28241c..37ca2f9 100644
> >> --- a/drivers/net/wireless/mwifiex/scan.c
> >> +++ b/drivers/net/wireless/mwifiex/scan.c
> >> @@ -1480,8 +1480,8 @@ mwifiex_update_curr_bss_params(struct mwifiex_private *priv,
> >> return -ENOMEM;
> >> }
> >> beacon_ie = kzalloc(ie_len, GFP_KERNEL);
> >> - if (!bss_desc) {
> >> - dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
> >> + if (!beacon_ie) {
> >> + dev_err(priv->adapter->dev, " failed to alloc beacon_ie\n");
> >> return -ENOMEM;
> >> }
> >> memcpy(beacon_ie, ie_buf, ie_len);
> > --
>
>
> this looks like a case for kmemdup()
Hi Walter,
You are right. I believe there are some more cases where kmemdup can be used.
Hi John,
Please apply Julia's patch as I will send a separate patch later to address Walter's comment. Thanks.
>
> just my 2 cents,
Thanks a lot for your 2 cents.
Bing
>
> re,
> wh
>
Hello all
On 08/22/2011 04:00 PM, Julia Lawall wrote:
> - if (!bss_desc) {
> + if (!beacon_ie) {
> dev_err(priv->adapter->dev, " failed to alloc bss_desc\n");
Shouldn't we also modify the error message, from "failed to alloc
bss_desc" to "failed to alloc beacon_ie" ?
Sincerely
Pierre Louis Aublin