* Re: [patch] staging: r8821ae: a couple macro expansion bugs
2014-01-28 14:00 [patch] staging: r8821ae: a couple macro expansion bugs Dan Carpenter
@ 2014-01-28 14:14 ` walter harms
2014-01-28 14:46 ` Dan Carpenter
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: walter harms @ 2014-01-28 14:14 UTC (permalink / raw)
To: kernel-janitors
Am 28.01.2014 15:00, schrieb Dan Carpenter:
> These macros need parentheses, otherwise it causes a macro expansion bug
> when they are used like this:
>
> ch->flags &= ~IEEE80211_CHAN_NO_IBSS;
>
> This was found using Smatch:
> drivers/staging/rtl8821ae/regd.c:200 _rtl_reg_apply_beaconing_flags()
> warn: the 'IEEE80211_CHAN_NO_IBSS' macro might need parens
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/rtl8821ae/regd.h b/drivers/staging/rtl8821ae/regd.h
> index abc60ab8165c..dceb3f18200b 100644
> --- a/drivers/staging/rtl8821ae/regd.h
> +++ b/drivers/staging/rtl8821ae/regd.h
> @@ -30,8 +30,8 @@
> #ifndef __RTL_REGD_H__
> #define __RTL_REGD_H__
>
> -#define IEEE80211_CHAN_NO_IBSS 1<<2
> -#define IEEE80211_CHAN_PASSIVE_SCAN 1<<1
> +#define IEEE80211_CHAN_NO_IBSS (1 << 2)
> +#define IEEE80211_CHAN_PASSIVE_SCAN (1 << 1)
> #define WIPHY_FLAG_CUSTOM_REGULATORY BIT(0)
> #define WIPHY_FLAG_STRICT_REGULATORY BIT(1)
> #define WIPHY_FLAG_DISABLE_BEACON_HINTS BIT(2)
> --
just one minor hint ...
could we settle for a common semantic here either 1<<2 or BIT(2) ?
just my 2 cents,
re,
wh
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] staging: r8821ae: a couple macro expansion bugs
2014-01-28 14:00 [patch] staging: r8821ae: a couple macro expansion bugs Dan Carpenter
2014-01-28 14:14 ` walter harms
@ 2014-01-28 14:46 ` Dan Carpenter
2014-01-28 15:01 ` Greg Kroah-Hartman
2014-01-28 16:25 ` Larry Finger
3 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2014-01-28 14:46 UTC (permalink / raw)
To: kernel-janitors
On Tue, Jan 28, 2014 at 03:14:21PM +0100, walter harms wrote:
>
>
> Am 28.01.2014 15:00, schrieb Dan Carpenter:
> > These macros need parentheses, otherwise it causes a macro expansion bug
> > when they are used like this:
> >
> > ch->flags &= ~IEEE80211_CHAN_NO_IBSS;
> >
> > This was found using Smatch:
> > drivers/staging/rtl8821ae/regd.c:200 _rtl_reg_apply_beaconing_flags()
> > warn: the 'IEEE80211_CHAN_NO_IBSS' macro might need parens
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/drivers/staging/rtl8821ae/regd.h b/drivers/staging/rtl8821ae/regd.h
> > index abc60ab8165c..dceb3f18200b 100644
> > --- a/drivers/staging/rtl8821ae/regd.h
> > +++ b/drivers/staging/rtl8821ae/regd.h
> > @@ -30,8 +30,8 @@
> > #ifndef __RTL_REGD_H__
> > #define __RTL_REGD_H__
> >
> > -#define IEEE80211_CHAN_NO_IBSS 1<<2
> > -#define IEEE80211_CHAN_PASSIVE_SCAN 1<<1
> > +#define IEEE80211_CHAN_NO_IBSS (1 << 2)
> > +#define IEEE80211_CHAN_PASSIVE_SCAN (1 << 1)
> > #define WIPHY_FLAG_CUSTOM_REGULATORY BIT(0)
> > #define WIPHY_FLAG_STRICT_REGULATORY BIT(1)
> > #define WIPHY_FLAG_DISABLE_BEACON_HINTS BIT(2)
> > --
>
>
> just one minor hint ...
> could we settle for a common semantic here either 1<<2 or BIT(2) ?
>
Yeah... This driver needs a lot of cleanup in later patches.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] staging: r8821ae: a couple macro expansion bugs
2014-01-28 14:00 [patch] staging: r8821ae: a couple macro expansion bugs Dan Carpenter
2014-01-28 14:14 ` walter harms
2014-01-28 14:46 ` Dan Carpenter
@ 2014-01-28 15:01 ` Greg Kroah-Hartman
2014-01-28 16:25 ` Larry Finger
3 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2014-01-28 15:01 UTC (permalink / raw)
To: kernel-janitors
On Tue, Jan 28, 2014 at 05:46:30PM +0300, Dan Carpenter wrote:
> On Tue, Jan 28, 2014 at 03:14:21PM +0100, walter harms wrote:
> >
> >
> > Am 28.01.2014 15:00, schrieb Dan Carpenter:
> > > These macros need parentheses, otherwise it causes a macro expansion bug
> > > when they are used like this:
> > >
> > > ch->flags &= ~IEEE80211_CHAN_NO_IBSS;
> > >
> > > This was found using Smatch:
> > > drivers/staging/rtl8821ae/regd.c:200 _rtl_reg_apply_beaconing_flags()
> > > warn: the 'IEEE80211_CHAN_NO_IBSS' macro might need parens
> > >
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > >
> > > diff --git a/drivers/staging/rtl8821ae/regd.h b/drivers/staging/rtl8821ae/regd.h
> > > index abc60ab8165c..dceb3f18200b 100644
> > > --- a/drivers/staging/rtl8821ae/regd.h
> > > +++ b/drivers/staging/rtl8821ae/regd.h
> > > @@ -30,8 +30,8 @@
> > > #ifndef __RTL_REGD_H__
> > > #define __RTL_REGD_H__
> > >
> > > -#define IEEE80211_CHAN_NO_IBSS 1<<2
> > > -#define IEEE80211_CHAN_PASSIVE_SCAN 1<<1
> > > +#define IEEE80211_CHAN_NO_IBSS (1 << 2)
> > > +#define IEEE80211_CHAN_PASSIVE_SCAN (1 << 1)
> > > #define WIPHY_FLAG_CUSTOM_REGULATORY BIT(0)
> > > #define WIPHY_FLAG_STRICT_REGULATORY BIT(1)
> > > #define WIPHY_FLAG_DISABLE_BEACON_HINTS BIT(2)
> > > --
> >
> >
> > just one minor hint ...
> > could we settle for a common semantic here either 1<<2 or BIT(2) ?
> >
>
> Yeah... This driver needs a lot of cleanup in later patches.
That will happen soon, for 3.15, in the netdev tree, but for 3.14 we
want this out there as people have the hardware and want to use it (i.e.
me and 1000 others that got the hardware at a recent conference.)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] staging: r8821ae: a couple macro expansion bugs
2014-01-28 14:00 [patch] staging: r8821ae: a couple macro expansion bugs Dan Carpenter
` (2 preceding siblings ...)
2014-01-28 15:01 ` Greg Kroah-Hartman
@ 2014-01-28 16:25 ` Larry Finger
3 siblings, 0 replies; 5+ messages in thread
From: Larry Finger @ 2014-01-28 16:25 UTC (permalink / raw)
To: kernel-janitors
On 01/28/2014 09:01 AM, Greg Kroah-Hartman wrote:
> On Tue, Jan 28, 2014 at 05:46:30PM +0300, Dan Carpenter wrote:
>> On Tue, Jan 28, 2014 at 03:14:21PM +0100, walter harms wrote:
>>>
>>>
>>> Am 28.01.2014 15:00, schrieb Dan Carpenter:
>>>> These macros need parentheses, otherwise it causes a macro expansion bug
>>>> when they are used like this:
>>>>
>>>> ch->flags &= ~IEEE80211_CHAN_NO_IBSS;
>>>>
>>>> This was found using Smatch:
>>>> drivers/staging/rtl8821ae/regd.c:200 _rtl_reg_apply_beaconing_flags()
>>>> warn: the 'IEEE80211_CHAN_NO_IBSS' macro might need parens
>>>>
>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>>
>>>> diff --git a/drivers/staging/rtl8821ae/regd.h b/drivers/staging/rtl8821ae/regd.h
>>>> index abc60ab8165c..dceb3f18200b 100644
>>>> --- a/drivers/staging/rtl8821ae/regd.h
>>>> +++ b/drivers/staging/rtl8821ae/regd.h
>>>> @@ -30,8 +30,8 @@
>>>> #ifndef __RTL_REGD_H__
>>>> #define __RTL_REGD_H__
>>>>
>>>> -#define IEEE80211_CHAN_NO_IBSS 1<<2
>>>> -#define IEEE80211_CHAN_PASSIVE_SCAN 1<<1
>>>> +#define IEEE80211_CHAN_NO_IBSS (1 << 2)
>>>> +#define IEEE80211_CHAN_PASSIVE_SCAN (1 << 1)
>>>> #define WIPHY_FLAG_CUSTOM_REGULATORY BIT(0)
>>>> #define WIPHY_FLAG_STRICT_REGULATORY BIT(1)
>>>> #define WIPHY_FLAG_DISABLE_BEACON_HINTS BIT(2)
>>>> --
>>>
>>>
>>> just one minor hint ...
>>> could we settle for a common semantic here either 1<<2 or BIT(2) ?
>>>
>>
>> Yeah... This driver needs a lot of cleanup in later patches.
>
> That will happen soon, for 3.15, in the netdev tree, but for 3.14 we
> want this out there as people have the hardware and want to use it (i.e.
> me and 1000 others that got the hardware at a recent conference.)
Please clean this up as much as you want, but be aware that this is a temporary
solution that was done in a hurry to handle some inconsistencies between
staging-next and linux-next. Neither Smatch nor Sparse were run on that version.
I suspect there are also endian issues that have not been repaired. I will be
reading such E-mails and will include your fixes, but the major cleanup is being
done on separate source.
Larry
^ permalink raw reply [flat|nested] 5+ messages in thread