checkpatch false positive

checkpatch false positive

[Richard Kennedy]

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

Re: checkpatch false positive

[Andy Whitcroft]

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

Re: checkpatch false positive

[Joe Perches]

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?

Re: checkpatch false positive

[Richard Kennedy]

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

[PATCH] wireless.h: Add STD_IW_HANDLER macro

[Joe Perches]

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))

Re: [PATCH] wireless.h: Add STD_IW_HANDLER macro

[Johannes Berg]

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

[PATCH 1/2] net/wireless/wext_core.c: Use IW_IOCTL_IDX macro

[Joe Perches]

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);

[PATCH 2/2] net/wireless/wext_core.c: Use IW_EVENT_IDX macro

[Joe Perches]

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]);
 	}

checkpatch false positive.

[Dave Jones]

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

Re: checkpatch false positive

[Joe Perches]

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.

Re: checkpatch false positive

[Hans de Goede]

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

Checkpatch: False positive

[Viresh Kumar]

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

Re: Checkpatch: False positive

[Joe Perches]

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.

Re: Checkpatch: False positive

[Andy Whitcroft]

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

Re: Checkpatch: False positive

[Joe Perches]

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*\|/) {

Re: Checkpatch: False positive

[Andy Whitcroft]

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