* checkpatch false positive
@ 2010-03-17 11:00 Richard Kennedy
2010-03-17 11:53 ` Andy Whitcroft
2010-03-17 15:25 ` Joe Perches
0 siblings, 2 replies; 16+ messages in thread
From: Richard Kennedy @ 2010-03-17 11:00 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: lkml
I'm getting this error from checkpatch which is a false positive AFAICT.
I don't see any other way to code this macro so maybe this rule
shouldn't apply?.
ERROR: space prohibited before open square bracket '['
#24: FILE: drivers/staging/wlan-ng/p80211wext.c:1685:
+#define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
regards
Richard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: checkpatch false positive
2010-03-17 11:00 checkpatch false positive Richard Kennedy
@ 2010-03-17 11:53 ` Andy Whitcroft
2010-03-17 15:25 ` Joe Perches
1 sibling, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2010-03-17 11:53 UTC (permalink / raw)
To: Richard Kennedy; +Cc: lkml
On Wed, Mar 17, 2010 at 11:00:29AM +0000, Richard Kennedy wrote:
> I'm getting this error from checkpatch which is a false positive AFAICT.
> I don't see any other way to code this macro so maybe this rule
> shouldn't apply?.
>
> ERROR: space prohibited before open square bracket '['
> #24: FILE: drivers/staging/wlan-ng/p80211wext.c:1685:
> +#define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
Yes that is definatly false, feed free to ignore it. I'll have a look
at fixing it.
-apw
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: checkpatch false positive
2010-03-17 11:00 checkpatch false positive Richard Kennedy
2010-03-17 11:53 ` Andy Whitcroft
@ 2010-03-17 15:25 ` Joe Perches
2010-03-17 15:40 ` Richard Kennedy
1 sibling, 1 reply; 16+ messages in thread
From: Joe Perches @ 2010-03-17 15:25 UTC (permalink / raw)
To: Richard Kennedy; +Cc: Andy Whitcroft, lkml
On Wed, 2010-03-17 at 11:00 +0000, Richard Kennedy wrote:
> I'm getting this error from checkpatch which is a false positive AFAICT.
> I don't see any other way to code this macro so maybe this rule
> shouldn't apply?.
>
> ERROR: space prohibited before open square bracket '['
> #24: FILE: drivers/staging/wlan-ng/p80211wext.c:1685:
> +#define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
While true that this is a false positive, hiding array indexing
brackets in a macro doesn't seem a good idea.
Maybe it'd be better to move the brackets to the use?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: checkpatch false positive
2010-03-17 15:25 ` Joe Perches
@ 2010-03-17 15:40 ` Richard Kennedy
2010-03-17 16:05 ` [PATCH] wireless.h: Add STD_IW_HANDLER macro Joe Perches
0 siblings, 1 reply; 16+ messages in thread
From: Richard Kennedy @ 2010-03-17 15:40 UTC (permalink / raw)
To: Joe Perches; +Cc: Andy Whitcroft, lkml
On Wed, 2010-03-17 at 08:25 -0700, Joe Perches wrote:
> On Wed, 2010-03-17 at 11:00 +0000, Richard Kennedy wrote:
> > I'm getting this error from checkpatch which is a false positive AFAICT.
> > I don't see any other way to code this macro so maybe this rule
> > shouldn't apply?.
> >
> > ERROR: space prohibited before open square bracket '['
> > #24: FILE: drivers/staging/wlan-ng/p80211wext.c:1685:
> > +#define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
>
> While true that this is a false positive, hiding array indexing
> brackets in a macro doesn't seem a good idea.
>
> Maybe it'd be better to move the brackets to the use?
>
>
err maybe ;)
I copied it from driver/net/wireless/ipw2x00/ipw2200.c
It just reduces typing when initialising the array :-
#define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
static iw_handler p80211wext_handlers[] = {
IW_IOCTL(SIOCSIWCOMMIT) = (iw_handler) p80211wext_siwcommit,
...
Oh, having quickly looked at wireless.h, I see there is already a macro
IW_IOCTL_IDX so I guess I should have used that!
would something like this be better?
static iw_handler p80211wext_handlers[] = {
[IW_IOCTL_IDX(SIOCSIWCOMMIT)] = (iw_handler) p80211wext_siwcommit,
...
regards
Richard
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] wireless.h: Add STD_IW_HANDLER macro
2010-03-17 15:40 ` Richard Kennedy
@ 2010-03-17 16:05 ` Joe Perches
2010-03-17 16:08 ` Johannes Berg
0 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2010-03-17 16:05 UTC (permalink / raw)
To: Richard Kennedy
Cc: Andy Whitcroft, gregkh, devel, linux-kernel, John W. Linville,
linux-wireless
On Wed, 2010-03-17 at 15:40 +0000, Richard Kennedy wrote:
> On Wed, 2010-03-17 at 08:25 -0700, Joe Perches wrote:
> > On Wed, 2010-03-17 at 11:00 +0000, Richard Kennedy wrote:
> > > I'm getting this error from checkpatch which is a false positive AFAICT.
> > > I don't see any other way to code this macro so maybe this rule
> > > shouldn't apply?.
> > > ERROR: space prohibited before open square bracket '['
> > > #24: FILE: drivers/staging/wlan-ng/p80211wext.c:1685:
> > > +#define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
> > While true that this is a false positive, hiding array indexing
> > brackets in a macro doesn't seem a good idea.
> > Maybe it'd be better to move the brackets to the use?
> err maybe ;)
> I copied it from driver/net/wireless/ipw2x00/ipw2200.c
> It just reduces typing when initialising the array :-
> #define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
> static iw_handler p80211wext_handlers[] = {
> IW_IOCTL(SIOCSIWCOMMIT) = (iw_handler) p80211wext_siwcommit,
> ...
> Oh, having quickly looked at wireless.h, I see there is already a macro
> IW_IOCTL_IDX so I guess I should have used that!
> would something like this be better?
> static iw_handler p80211wext_handlers[] = {
> [IW_IOCTL_IDX(SIOCSIWCOMMIT)] = (iw_handler) p80211wext_siwcommit,
> ...
orinoco uses one more macro indirection which seems reasonable
Maybe this define should be moved to wireless.h
#define STD_IW_HANDLER(id, func) \
[IW_IOCTL_IDX(id)] = (iw_handler) func
the line continued macro probably avoids checkpatch warnings too...
orinoco use:
static const iw_handler orinoco_handler[] = {
STD_IW_HANDLER(SIOCSIWCOMMIT, orinoco_ioctl_commit),
STD_IW_HANDLER(SIOCGIWNAME, cfg80211_wext_giwname),
Here's a suggested patch:
Signed-off-by: Joe Perches <joe@perches.com>
---
diff --git a/include/linux/wireless.h b/include/linux/wireless.h
index 5b4c6c7..ad9f8d5 100644
--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -346,6 +346,8 @@
#define SIOCIWFIRST 0x8B00
#define SIOCIWLAST SIOCIWLASTPRIV /* 0x8BFF */
#define IW_IOCTL_IDX(cmd) ((cmd) - SIOCIWFIRST)
+#define STD_IW_HANDLER(id, func) \
+ [IW_IOCTL_IDX(id)] = (iw_handler) func
/* Odd : get (world access), even : set (root access) */
#define IW_IS_SET(cmd) (!((cmd) & 0x1))
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] wireless.h: Add STD_IW_HANDLER macro
2010-03-17 16:05 ` [PATCH] wireless.h: Add STD_IW_HANDLER macro Joe Perches
@ 2010-03-17 16:08 ` Johannes Berg
2010-03-17 17:46 ` [PATCH 1/2] net/wireless/wext_core.c: Use IW_IOCTL_IDX macro Joe Perches
0 siblings, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2010-03-17 16:08 UTC (permalink / raw)
To: Joe Perches
Cc: Richard Kennedy, Andy Whitcroft, gregkh, devel, linux-kernel,
John W. Linville, linux-wireless
On Wed, 2010-03-17 at 09:05 -0700, Joe Perches wrote:
> > I copied it from driver/net/wireless/ipw2x00/ipw2200.c
> > It just reduces typing when initialising the array :-
> > #define IW_IOCTL(x) [(x)-SIOCSIWCOMMIT]
> > static iw_handler p80211wext_handlers[] = {
> > IW_IOCTL(SIOCSIWCOMMIT) = (iw_handler) p80211wext_siwcommit,
> > ...
> > Oh, having quickly looked at wireless.h, I see there is already a macro
> > IW_IOCTL_IDX so I guess I should have used that!
> > would something like this be better?
> > static iw_handler p80211wext_handlers[] = {
> > [IW_IOCTL_IDX(SIOCSIWCOMMIT)] = (iw_handler) p80211wext_siwcommit,
> > ...
>
> orinoco uses one more macro indirection which seems reasonable
>
> Maybe this define should be moved to wireless.h
>
> #define STD_IW_HANDLER(id, func) \
> [IW_IOCTL_IDX(id)] = (iw_handler) func
>
> the line continued macro probably avoids checkpatch warnings too...
>
> orinoco use:
>
> static const iw_handler orinoco_handler[] = {
> STD_IW_HANDLER(SIOCSIWCOMMIT, orinoco_ioctl_commit),
> STD_IW_HANDLER(SIOCGIWNAME, cfg80211_wext_giwname),
>
> Here's a suggested patch:
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> diff --git a/include/linux/wireless.h b/include/linux/wireless.h
> index 5b4c6c7..ad9f8d5 100644
> --- a/include/linux/wireless.h
> +++ b/include/linux/wireless.h
> @@ -346,6 +346,8 @@
> #define SIOCIWFIRST 0x8B00
> #define SIOCIWLAST SIOCIWLASTPRIV /* 0x8BFF */
> #define IW_IOCTL_IDX(cmd) ((cmd) - SIOCIWFIRST)
> +#define STD_IW_HANDLER(id, func) \
> + [IW_IOCTL_IDX(id)] = (iw_handler) func
Err, who still cares about wireless extensions (who isn't working on
net/wireless/wext*)?
johannes
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] net/wireless/wext_core.c: Use IW_IOCTL_IDX macro
2010-03-17 16:08 ` Johannes Berg
@ 2010-03-17 17:46 ` Joe Perches
2010-03-17 17:48 ` [PATCH 2/2] net/wireless/wext_core.c: Use IW_EVENT_IDX macro Joe Perches
0 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2010-03-17 17:46 UTC (permalink / raw)
To: Johannes Berg
Cc: Richard Kennedy, Andy Whitcroft, gregkh, devel, linux-kernel,
John W. Linville, linux-wireless
There's a wireless.h macro for this, might as well use it.
Signed-off-by: Joe Perches <joe@perches.com>
---
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index 5e1656b..dbde22b 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -28,226 +28,226 @@ typedef int (*wext_ioctl_func)(struct net_device *, struct iwreq *,
* know about.
*/
static const struct iw_ioctl_description standard_ioctl[] = {
- [SIOCSIWCOMMIT - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWCOMMIT)] = {
.header_type = IW_HEADER_TYPE_NULL,
},
- [SIOCGIWNAME - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWNAME)] = {
.header_type = IW_HEADER_TYPE_CHAR,
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWNWID - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWNWID)] = {
.header_type = IW_HEADER_TYPE_PARAM,
.flags = IW_DESCR_FLAG_EVENT,
},
- [SIOCGIWNWID - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWNWID)] = {
.header_type = IW_HEADER_TYPE_PARAM,
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWFREQ - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWFREQ)] = {
.header_type = IW_HEADER_TYPE_FREQ,
.flags = IW_DESCR_FLAG_EVENT,
},
- [SIOCGIWFREQ - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWFREQ)] = {
.header_type = IW_HEADER_TYPE_FREQ,
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWMODE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWMODE)] = {
.header_type = IW_HEADER_TYPE_UINT,
.flags = IW_DESCR_FLAG_EVENT,
},
- [SIOCGIWMODE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWMODE)] = {
.header_type = IW_HEADER_TYPE_UINT,
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWSENS - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWSENS)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWSENS - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWSENS)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWRANGE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWRANGE)] = {
.header_type = IW_HEADER_TYPE_NULL,
},
- [SIOCGIWRANGE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWRANGE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = sizeof(struct iw_range),
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWPRIV - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWPRIV)] = {
.header_type = IW_HEADER_TYPE_NULL,
},
- [SIOCGIWPRIV - SIOCIWFIRST] = { /* (handled directly by us) */
+ [IW_IOCTL_IDX(SIOCGIWPRIV)] = { /* (handled directly by us) */
.header_type = IW_HEADER_TYPE_POINT,
.token_size = sizeof(struct iw_priv_args),
.max_tokens = 16,
.flags = IW_DESCR_FLAG_NOMAX,
},
- [SIOCSIWSTATS - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWSTATS)] = {
.header_type = IW_HEADER_TYPE_NULL,
},
- [SIOCGIWSTATS - SIOCIWFIRST] = { /* (handled directly by us) */
+ [IW_IOCTL_IDX(SIOCGIWSTATS)] = { /* (handled directly by us) */
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = sizeof(struct iw_statistics),
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWSPY - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWSPY)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = sizeof(struct sockaddr),
.max_tokens = IW_MAX_SPY,
},
- [SIOCGIWSPY - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWSPY)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = sizeof(struct sockaddr) +
sizeof(struct iw_quality),
.max_tokens = IW_MAX_SPY,
},
- [SIOCSIWTHRSPY - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWTHRSPY)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = sizeof(struct iw_thrspy),
.min_tokens = 1,
.max_tokens = 1,
},
- [SIOCGIWTHRSPY - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWTHRSPY)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = sizeof(struct iw_thrspy),
.min_tokens = 1,
.max_tokens = 1,
},
- [SIOCSIWAP - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWAP)] = {
.header_type = IW_HEADER_TYPE_ADDR,
},
- [SIOCGIWAP - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWAP)] = {
.header_type = IW_HEADER_TYPE_ADDR,
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWMLME - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWMLME)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.min_tokens = sizeof(struct iw_mlme),
.max_tokens = sizeof(struct iw_mlme),
},
- [SIOCGIWAPLIST - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWAPLIST)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = sizeof(struct sockaddr) +
sizeof(struct iw_quality),
.max_tokens = IW_MAX_AP,
.flags = IW_DESCR_FLAG_NOMAX,
},
- [SIOCSIWSCAN - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWSCAN)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.min_tokens = 0,
.max_tokens = sizeof(struct iw_scan_req),
},
- [SIOCGIWSCAN - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWSCAN)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_SCAN_MAX_DATA,
.flags = IW_DESCR_FLAG_NOMAX,
},
- [SIOCSIWESSID - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWESSID)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_ESSID_MAX_SIZE,
.flags = IW_DESCR_FLAG_EVENT,
},
- [SIOCGIWESSID - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWESSID)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_ESSID_MAX_SIZE,
.flags = IW_DESCR_FLAG_DUMP,
},
- [SIOCSIWNICKN - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWNICKN)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_ESSID_MAX_SIZE,
},
- [SIOCGIWNICKN - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWNICKN)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_ESSID_MAX_SIZE,
},
- [SIOCSIWRATE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWRATE)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWRATE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWRATE)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWRTS - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWRTS)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWRTS - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWRTS)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWFRAG - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWFRAG)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWFRAG - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWFRAG)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWTXPOW - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWTXPOW)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWTXPOW - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWTXPOW)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWRETRY - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWRETRY)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWRETRY - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWRETRY)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWENCODE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWENCODE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_ENCODING_TOKEN_MAX,
.flags = IW_DESCR_FLAG_EVENT | IW_DESCR_FLAG_RESTRICT,
},
- [SIOCGIWENCODE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWENCODE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_ENCODING_TOKEN_MAX,
.flags = IW_DESCR_FLAG_DUMP | IW_DESCR_FLAG_RESTRICT,
},
- [SIOCSIWPOWER - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWPOWER)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWPOWER - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWPOWER)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWGENIE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWGENIE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_GENERIC_IE_MAX,
},
- [SIOCGIWGENIE - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWGENIE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_GENERIC_IE_MAX,
},
- [SIOCSIWAUTH - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWAUTH)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCGIWAUTH - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWAUTH)] = {
.header_type = IW_HEADER_TYPE_PARAM,
},
- [SIOCSIWENCODEEXT - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWENCODEEXT)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.min_tokens = sizeof(struct iw_encode_ext),
.max_tokens = sizeof(struct iw_encode_ext) +
IW_ENCODING_TOKEN_MAX,
},
- [SIOCGIWENCODEEXT - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCGIWENCODEEXT)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.min_tokens = sizeof(struct iw_encode_ext),
.max_tokens = sizeof(struct iw_encode_ext) +
IW_ENCODING_TOKEN_MAX,
},
- [SIOCSIWPMKSA - SIOCIWFIRST] = {
+ [IW_IOCTL_IDX(SIOCSIWPMKSA)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.min_tokens = sizeof(struct iw_pmksa),
@@ -449,7 +449,7 @@ void wireless_send_event(struct net_device * dev,
/* Get the description of the Event */
if (cmd <= SIOCIWLAST) {
- cmd_index = cmd - SIOCIWFIRST;
+ cmd_index = IW_IOCTL_IDX(cmd);
if (cmd_index < standard_ioctl_num)
descr = &(standard_ioctl[cmd_index]);
} else {
@@ -662,7 +662,7 @@ static iw_handler get_handler(struct net_device *dev, unsigned int cmd)
return NULL;
/* Try as a standard command */
- index = cmd - SIOCIWFIRST;
+ index = IW_IOCTL_IDX(cmd);
if (index < handlers->num_standard)
return handlers->standard[index];
@@ -954,9 +954,9 @@ static int ioctl_standard_call(struct net_device * dev,
int ret = -EINVAL;
/* Get the description of the IOCTL */
- if ((cmd - SIOCIWFIRST) >= standard_ioctl_num)
+ if (IW_IOCTL_IDX(cmd) >= standard_ioctl_num)
return -EOPNOTSUPP;
- descr = &(standard_ioctl[cmd - SIOCIWFIRST]);
+ descr = &(standard_ioctl[IW_IOCTL_IDX(cmd)]);
/* Check if we have a pointer to user space data or not */
if (descr->header_type != IW_HEADER_TYPE_POINT) {
@@ -1012,7 +1012,7 @@ static int compat_standard_call(struct net_device *dev,
struct iw_point iwp;
int err;
- descr = standard_ioctl + (cmd - SIOCIWFIRST);
+ descr = standard_ioctl + IW_IOCTL_IDX(cmd);
if (descr->header_type != IW_HEADER_TYPE_POINT)
return ioctl_standard_call(dev, iwr, cmd, info, handler);
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] net/wireless/wext_core.c: Use IW_EVENT_IDX macro
2010-03-17 17:46 ` [PATCH 1/2] net/wireless/wext_core.c: Use IW_IOCTL_IDX macro Joe Perches
@ 2010-03-17 17:48 ` Joe Perches
0 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2010-03-17 17:48 UTC (permalink / raw)
To: Johannes Berg
Cc: Richard Kennedy, Andy Whitcroft, gregkh, devel, linux-kernel,
John W. Linville, linux-wireless
There's a wireless.h macro for this, might as well use it.
Signed-off-by: Joe Perches <joe@perches.com>
---
diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c
index dbde22b..bfcbeee 100644
--- a/net/wireless/wext-core.c
+++ b/net/wireless/wext-core.c
@@ -261,44 +261,44 @@ static const unsigned standard_ioctl_num = ARRAY_SIZE(standard_ioctl);
* we know about.
*/
static const struct iw_ioctl_description standard_event[] = {
- [IWEVTXDROP - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVTXDROP)] = {
.header_type = IW_HEADER_TYPE_ADDR,
},
- [IWEVQUAL - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVQUAL)] = {
.header_type = IW_HEADER_TYPE_QUAL,
},
- [IWEVCUSTOM - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVCUSTOM)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_CUSTOM_MAX,
},
- [IWEVREGISTERED - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVREGISTERED)] = {
.header_type = IW_HEADER_TYPE_ADDR,
},
- [IWEVEXPIRED - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVEXPIRED)] = {
.header_type = IW_HEADER_TYPE_ADDR,
},
- [IWEVGENIE - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVGENIE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_GENERIC_IE_MAX,
},
- [IWEVMICHAELMICFAILURE - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVMICHAELMICFAILURE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = sizeof(struct iw_michaelmicfailure),
},
- [IWEVASSOCREQIE - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVASSOCREQIE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_GENERIC_IE_MAX,
},
- [IWEVASSOCRESPIE - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVASSOCRESPIE)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = IW_GENERIC_IE_MAX,
},
- [IWEVPMKIDCAND - IWEVFIRST] = {
+ [IW_EVENT_IDX(IWEVPMKIDCAND)] = {
.header_type = IW_HEADER_TYPE_POINT,
.token_size = 1,
.max_tokens = sizeof(struct iw_pmkid_cand),
@@ -453,7 +453,7 @@ void wireless_send_event(struct net_device * dev,
if (cmd_index < standard_ioctl_num)
descr = &(standard_ioctl[cmd_index]);
} else {
- cmd_index = cmd - IWEVFIRST;
+ cmd_index = IW_EVENT_IDX(cmd);
if (cmd_index < standard_event_num)
descr = &(standard_event[cmd_index]);
}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* checkpatch false positive.
@ 2010-08-11 16:35 Dave Jones
0 siblings, 0 replies; 16+ messages in thread
From: Dave Jones @ 2010-08-11 16:35 UTC (permalink / raw)
To: apw; +Cc: Linux Kernel
I just got this from a patch I merged..
ERROR: need consistent spacing around '*' (ctx:WxV)
#121: FILE: arch/x86/kernel/cpu/cpufreq/pcc-cpufreq.c:113:
+static struct pcc_cpu __percpu *pcc_cpu_info;
^
which doesn't seem right.
Dave
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: checkpatch false positive
[not found] <54461602.4000705@redhat.com>
@ 2014-10-21 8:28 ` Joe Perches
2014-10-21 9:27 ` Hans de Goede
0 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2014-10-21 8:28 UTC (permalink / raw)
To: Hans de Goede; +Cc: Andy Whitcroft, LKML
On Tue, 2014-10-21 at 10:14 +0200, Hans de Goede wrote:
> Hi,
>
> Checkpatch gives the following warning:
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #31:
> new file mode 100644
>
> total: 0 errors, 1 warnings, 352 lines checked
>
> 0001-input-Add-new-sun4i-lradc-keys-driver.patch has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> On a patch of mine, even though it updates MAINTAINERS properly, it would
> be nice if checkpatch would check for a hunk updating MAINTAINERS, and then
> would not issue this warning (note my perl-foo is way too weak to fix this
> myself).
>
> I've attached the patch triggering the warning.
Hi Hans.
It's not really fixable. Of course you are welcome to
try though.
Many patches are discrete and the entire series isn't
visible to a single MAINTAINERS update scan by checkpatch.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: checkpatch false positive
2014-10-21 8:28 ` Joe Perches
@ 2014-10-21 9:27 ` Hans de Goede
0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2014-10-21 9:27 UTC (permalink / raw)
To: Joe Perches; +Cc: Andy Whitcroft, LKML
Hi,
On 10/21/2014 10:28 AM, Joe Perches wrote:
> On Tue, 2014-10-21 at 10:14 +0200, Hans de Goede wrote:
>> Hi,
>>
>> Checkpatch gives the following warning:
>>
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> #31:
>> new file mode 100644
>>
>> total: 0 errors, 1 warnings, 352 lines checked
>>
>> 0001-input-Add-new-sun4i-lradc-keys-driver.patch has style problems, please review.
>>
>> If any of these errors are false positives, please report
>> them to the maintainer, see CHECKPATCH in MAINTAINERS.
>>
>> On a patch of mine, even though it updates MAINTAINERS properly, it would
>> be nice if checkpatch would check for a hunk updating MAINTAINERS, and then
>> would not issue this warning (note my perl-foo is way too weak to fix this
>> myself).
>>
>> I've attached the patch triggering the warning.
>
> Hi Hans.
>
> It's not really fixable. Of course you are welcome to
> try though.
>
> Many patches are discrete and the entire series isn't
> visible to a single MAINTAINERS update scan by checkpatch.
I understand that in that scenario it is not fixable, but in the case I was
talking about the addition of the new file and the MAINTAINERS update
are part of a single patch (adding a small new driver), it would be nice
if at least in that case checkpatch would not complain. This does not even
need to be really smart, it could just check if a patch introducing new
files is touching MAINTAINERS at all, and if it does suppress the warning.
As said my perl-foo is weak, so I will not be trying to fix this myself.
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread
* Checkpatch: False positive
@ 2015-07-16 10:55 Viresh Kumar
2015-07-16 15:35 ` Joe Perches
0 siblings, 1 reply; 16+ messages in thread
From: Viresh Kumar @ 2015-07-16 10:55 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: linux-kernel
Hi Andy/Joe,
I got a warning today for my cover-letter, and it looked like a false
positive. Please have a look, based of v4.2-rc2.
-----------------------
0000-cover-letter.patch
-----------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#31:
arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
--
viresh
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Checkpatch: False positive
2015-07-16 10:55 Checkpatch: False positive Viresh Kumar
@ 2015-07-16 15:35 ` Joe Perches
2015-07-16 15:43 ` Andy Whitcroft
0 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2015-07-16 15:35 UTC (permalink / raw)
To: Viresh Kumar; +Cc: Andy Whitcroft, linux-kernel, Dan Carpenter, Andrew Morton
On Thu, 2015-07-16 at 16:25 +0530, Viresh Kumar wrote:
> Hi Andy/Joe,
>
> I got a warning today for my cover-letter, and it looked like a false
> positive. Please have a look, based of v4.2-rc2.
> -----------------------
> 0000-cover-letter.patch
> -----------------------
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #31:
> arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
There are a lot of other false positives for this test.
Maybe it's not worth checking for this condition and the
test should be removed as "fixing" it may not be feasible.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Checkpatch: False positive
2015-07-16 15:35 ` Joe Perches
@ 2015-07-16 15:43 ` Andy Whitcroft
2015-07-16 15:58 ` Joe Perches
0 siblings, 1 reply; 16+ messages in thread
From: Andy Whitcroft @ 2015-07-16 15:43 UTC (permalink / raw)
To: Joe Perches; +Cc: Viresh Kumar, linux-kernel, Dan Carpenter, Andrew Morton
On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > #31:
> > arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
I guess those are in the limbo land between the end of message and
beginning of the patch itself. Perhaps the test should at least stop at
the end of header marker, at the '---'.
-apw
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Checkpatch: False positive
2015-07-16 15:43 ` Andy Whitcroft
@ 2015-07-16 15:58 ` Joe Perches
2015-07-16 17:21 ` Andy Whitcroft
0 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2015-07-16 15:58 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Viresh Kumar, linux-kernel, Dan Carpenter, Andrew Morton
On Thu, 2015-07-16 at 16:43 +0100, Andy Whitcroft wrote:
> On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > > #31:
> > > arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
>
> I guess those are in the limbo land between the end of message and
> beginning of the patch itself. Perhaps the test should at least stop at
> the end of header marker, at the '---'.
>
> -apw
Maybe, but the test already stops at signatures like
Signed-off-by: that should always be above the ---.
This might help, but there are _many_ false positives.
The other thing that might help is for people to take
the warnings the script produces less seriously.
Maybe convert:
ERROR -> defect
WARNING -> unstylish
CHECK -> nitpick
or some such
---
scripts/checkpatch.pl | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d5ce29a..5e7afa7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2213,6 +2213,11 @@ sub process {
$in_commit_log = 0;
}
+# Check for patch separator
+ if ($line =~ /^---$/) {
+ $in_commit_log = 0;
+ }
+
# Check if MAINTAINERS is being updated. If so, there's probably no need to
# emit the "does MAINTAINERS need updating?" message on file add/move/delete
if ($line =~ /^\s*MAINTAINERS\s*\|/) {
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: Checkpatch: False positive
2015-07-16 15:58 ` Joe Perches
@ 2015-07-16 17:21 ` Andy Whitcroft
0 siblings, 0 replies; 16+ messages in thread
From: Andy Whitcroft @ 2015-07-16 17:21 UTC (permalink / raw)
To: Joe Perches; +Cc: Viresh Kumar, linux-kernel, Dan Carpenter, Andrew Morton
On Thu, Jul 16, 2015 at 08:58:56AM -0700, Joe Perches wrote:
> On Thu, 2015-07-16 at 16:43 +0100, Andy Whitcroft wrote:
> > On Thu, Jul 16, 2015 at 08:35:58AM -0700, Joe Perches wrote:
> > > > #31:
> > > > arch/x86/kernel/hpet.c | 198 ++++++++++++++++++++++++++---------------
> >
> > I guess those are in the limbo land between the end of message and
> > beginning of the patch itself. Perhaps the test should at least stop at
> > the end of header marker, at the '---'.
> >
> > -apw
>
> Maybe, but the test already stops at signatures like
> Signed-off-by: that should always be above the ---.
>
> This might help, but there are _many_ false positives.
>
> The other thing that might help is for people to take
> the warnings the script produces less seriously.
>
> Maybe convert:
>
> ERROR -> defect
> WARNING -> unstylish
> CHECK -> nitpick
Heh, that has long been the main issue, please please believe your brain
not checkpatch. But yes some less inflamitory words might, just might,
reduce the noise.
-apw
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-07-16 17:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-17 11:00 checkpatch false positive Richard Kennedy
2010-03-17 11:53 ` Andy Whitcroft
2010-03-17 15:25 ` Joe Perches
2010-03-17 15:40 ` Richard Kennedy
2010-03-17 16:05 ` [PATCH] wireless.h: Add STD_IW_HANDLER macro Joe Perches
2010-03-17 16:08 ` Johannes Berg
2010-03-17 17:46 ` [PATCH 1/2] net/wireless/wext_core.c: Use IW_IOCTL_IDX macro Joe Perches
2010-03-17 17:48 ` [PATCH 2/2] net/wireless/wext_core.c: Use IW_EVENT_IDX macro Joe Perches
-- strict thread matches above, loose matches on Subject: below --
2010-08-11 16:35 checkpatch false positive Dave Jones
[not found] <54461602.4000705@redhat.com>
2014-10-21 8:28 ` Joe Perches
2014-10-21 9:27 ` Hans de Goede
2015-07-16 10:55 Checkpatch: False positive Viresh Kumar
2015-07-16 15:35 ` Joe Perches
2015-07-16 15:43 ` Andy Whitcroft
2015-07-16 15:58 ` Joe Perches
2015-07-16 17:21 ` Andy Whitcroft
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox