* 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread
* checkpatch false positive.
@ 2010-08-11 16:35 Dave Jones
0 siblings, 0 replies; 20+ 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] 20+ messages in thread[parent not found: <54461602.4000705@redhat.com>]
* 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; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread
* Checkpatch: False positive @ 2015-07-16 10:55 Viresh Kumar 2015-07-16 15:35 ` Joe Perches 0 siblings, 1 reply; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ 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; 20+ 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] 20+ messages in thread
* Checkpatch False positive
@ 2018-08-20 20:21 Jeff Kirsher
2018-08-20 20:22 ` Matthew Wilcox
0 siblings, 1 reply; 20+ messages in thread
From: Jeff Kirsher @ 2018-08-20 20:21 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: netdev, Linux Docs
[-- Attachment #1: Type: text/plain, Size: 845 bytes --]
Checkpatch.pl should probably exclude *.rst files from the SPDX License
Identifier check, since they are documentational files, like *.txt.
Here is the warning message I got on a recent documentation patch I
created.
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#28: FILE: Documentation/networking/fm10k.rst:1:
+Linux* Base Driver for Intel(R) Ethernet Multi-host Controller
total: 0 errors, 1 warnings, 0 checks, 162 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-
inplace.
.apply/v4-Documentation-fm10k-Add-kernel-documentation.patch has style
problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: Checkpatch False positive 2018-08-20 20:21 Checkpatch " Jeff Kirsher @ 2018-08-20 20:22 ` Matthew Wilcox 2018-08-20 20:32 ` Jeff Kirsher 0 siblings, 1 reply; 20+ messages in thread From: Matthew Wilcox @ 2018-08-20 20:22 UTC (permalink / raw) To: Jeff Kirsher; +Cc: Andy Whitcroft, Joe Perches, netdev, Linux Docs On Mon, Aug 20, 2018 at 01:21:30PM -0700, Jeff Kirsher wrote: > Checkpatch.pl should probably exclude *.rst files from the SPDX License > Identifier check, since they are documentational files, like *.txt. They're still copyrighted work, so they should still have SPDX tags to identify their license. checkpatch has: } elsif ($realfile =~ /\.rst$/) { $comment = '..'; so we're explicitly including rst files in the checking for now. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Checkpatch False positive 2018-08-20 20:22 ` Matthew Wilcox @ 2018-08-20 20:32 ` Jeff Kirsher 0 siblings, 0 replies; 20+ messages in thread From: Jeff Kirsher @ 2018-08-20 20:32 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Andy Whitcroft, Joe Perches, netdev, Linux Docs [-- Attachment #1: Type: text/plain, Size: 735 bytes --] On Mon, 2018-08-20 at 13:22 -0700, Matthew Wilcox wrote: > On Mon, Aug 20, 2018 at 01:21:30PM -0700, Jeff Kirsher wrote: > > Checkpatch.pl should probably exclude *.rst files from the SPDX > > License > > Identifier check, since they are documentational files, like > > *.txt. > > They're still copyrighted work, so they should still have SPDX tags > to identify their license. > > checkpatch has: > > } elsif ($realfile =~ /\.rst$/) { > $comment = '..'; > > so we're explicitly including rst files in the checking for now. Ah, sorry. I did not catch that they started to include SPDX-License- Identifiers for RST files, thanks Matthew. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Checkpatch false positive
@ 2026-10-29 15:50 Dave Stevenson
0 siblings, 0 replies; 20+ messages in thread
From: Dave Stevenson @ 2026-10-29 15:50 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: LKML
Hi Andy and Joe.
Checkpatch is giving the error
ERROR: missing sentinel in ID array
#1455: FILE: drivers/media/i2c/sony-starvis2.c:1450:
+static const struct of_device_id starvis2_of_match[] = {
{ .compatible = "XXXXXXXXXXXXXXXX", .data = &imx678_aamr_info },
{ .compatible = "XXXXXXXXXXXXXXXX", .data = &imx678_aaqr_info },
on patch https://lore.kernel.org/linux-media/20261022-media-starvis2-v6-1-374343f55fd3@raspberrypi.com/
The sentinel is there, but just not in that hunk that renames the
struct of_device_id. That counts as a false positive to me, hence I'm
reporting it to you as requested.
Thanks
Dave
^ permalink raw reply [flat|nested] 20+ messages in threadend of thread, other threads:[~2026-10-29 15:50 UTC | newest]
Thread overview: 20+ 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
2018-08-20 20:21 Checkpatch " Jeff Kirsher
2018-08-20 20:22 ` Matthew Wilcox
2018-08-20 20:32 ` Jeff Kirsher
2026-10-29 15:50 Checkpatch false positive Dave Stevenson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.