* [PATCH] checkpatch: Allow spaces before the ':' of a bitfield
[not found] ` <687af3bd-edc6-40cf-b994-0d0fdb7cd30d@googlegroups.com>
@ 2013-10-14 17:32 ` Josh Triplett
2013-10-14 17:51 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2013-10-14 17:32 UTC (permalink / raw)
To: linux-kernel
Cc: Nandini Hanumanthagowda, Greg Kroah-Hartman, Andy Whitcroft, Joe Perches
checkpatch warns about spaces both before and after the ':' separating a
bitfield name from its width. However, many drivers do put space before
the : to line up the widths, which makes the definition significantly
more readable; checkpatch should not warn about that. Remove the
warning for space before the ':' of a bitfield.
Reported-by: Nandini Hanumanthagowda <nandu.hgowda@gmail.com>
Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
Nandini and Greg observed this issue as part of checkpatch warnings on a
staging driver (octeon-usb), and both agreed that checkpatch should not
issue this warning; patching accordingly.
scripts/checkpatch.pl | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ed16a68..9e36345 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2934,8 +2934,7 @@ sub process {
# No spaces for:
# ->
- # : when part of a bitfield
- } elsif ($op eq '->' || $opv eq ':B') {
+ } elsif ($op eq '->') {
if ($ctx =~ /Wx.|.xW/) {
if (ERROR("SPACING",
"spaces prohibited around that '$op' $at\n" . $hereptr)) {
@@ -2947,6 +2946,19 @@ sub process {
}
}
+ # : when part of a bitfield
+ } elsif ($opv eq ':B') {
+ if ($ctx =~ /.xW/) {
+ if (ERROR("SPACING",
+ "spaces prohibited after that '$op' $at\n" . $hereptr)) {
+ $good = $fix_elements[$n] . rtrim($fix_elements[$n + 1]);
+ if (defined $fix_elements[$n + 2]) {
+ $fix_elements[$n + 2] =~ s/^\s+//;
+ }
+ $line_fixed = 1;
+ }
+ }
+
# , must have a space on the right.
} elsif ($op eq ',') {
if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) {
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Allow spaces before the ':' of a bitfield
2013-10-14 17:32 ` [PATCH] checkpatch: Allow spaces before the ':' of a bitfield Josh Triplett
@ 2013-10-14 17:51 ` Joe Perches
2013-10-14 18:00 ` Josh Triplett
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-10-14 17:51 UTC (permalink / raw)
To: Josh Triplett
Cc: linux-kernel, Nandini Hanumanthagowda, Greg Kroah-Hartman,
Andy Whitcroft
On Mon, 2013-10-14 at 10:32 -0700, Josh Triplett wrote:
> checkpatch warns about spaces both before and after the ':' separating a
> bitfield name from its width. However, many drivers do put space before
> the : to line up the widths, which makes the definition significantly
> more readable; checkpatch should not warn about that. Remove the
> warning for space before the ':' of a bitfield.
There are a _bunch_ of different styles used for
alignment of bitfields.
$ grep -rPh --include=*.[ch] \
'(?:unsigned\s+|signed\s+|)(char|short|long|long\s+long|\w+_t|(?:__|)(?:u|s)(?:8|16|32|64))\s+\w+\s*:\s*\d+\s*(?:,|;)' *
spaces before colon
spaces after colon
aligned on bitfield size
I'd just as soon make it a --strict check.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Allow spaces before the ':' of a bitfield
2013-10-14 17:51 ` Joe Perches
@ 2013-10-14 18:00 ` Josh Triplett
2013-10-14 18:04 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2013-10-14 18:00 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Nandini Hanumanthagowda, Greg Kroah-Hartman,
Andy Whitcroft
On Mon, Oct 14, 2013 at 10:51:48AM -0700, Joe Perches wrote:
> On Mon, 2013-10-14 at 10:32 -0700, Josh Triplett wrote:
> > checkpatch warns about spaces both before and after the ':' separating a
> > bitfield name from its width. However, many drivers do put space before
> > the : to line up the widths, which makes the definition significantly
> > more readable; checkpatch should not warn about that. Remove the
> > warning for space before the ':' of a bitfield.
>
> There are a _bunch_ of different styles used for
> alignment of bitfields.
>
> $ grep -rPh --include=*.[ch] \
> '(?:unsigned\s+|signed\s+|)(char|short|long|long\s+long|\w+_t|(?:__|)(?:u|s)(?:8|16|32|64))\s+\w+\s*:\s*\d+\s*(?:,|;)' *
>
> spaces before colon
> spaces after colon
> aligned on bitfield size
>
> I'd just as soon make it a --strict check.
I'd be fine with removing the check entirely, or making it --strict;
however, even for --strict mode I'd suggest applying this patch, as I
don't think checkpatch should ever prohibit spaces before the ':'.
- Josh Triplett
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Allow spaces before the ':' of a bitfield
2013-10-14 18:00 ` Josh Triplett
@ 2013-10-14 18:04 ` Joe Perches
2013-10-14 20:20 ` Josh Triplett
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2013-10-14 18:04 UTC (permalink / raw)
To: Josh Triplett
Cc: linux-kernel, Nandini Hanumanthagowda, Greg Kroah-Hartman,
Andy Whitcroft
On Mon, 2013-10-14 at 11:00 -0700, Josh Triplett wrote:
> On Mon, Oct 14, 2013 at 10:51:48AM -0700, Joe Perches wrote:
> > On Mon, 2013-10-14 at 10:32 -0700, Josh Triplett wrote:
> > > checkpatch warns about spaces both before and after the ':' separating a
> > > bitfield name from its width. However, many drivers do put space before
> > > the : to line up the widths, which makes the definition significantly
> > > more readable; checkpatch should not warn about that. Remove the
> > > warning for space before the ':' of a bitfield.
> >
> > There are a _bunch_ of different styles used for
> > alignment of bitfields.
> >
> > $ grep -rPh --include=*.[ch] \
> > '(?:unsigned\s+|signed\s+|)(char|short|long|long\s+long|\w+_t|(?:__|)(?:u|s)(?:8|16|32|64))\s+\w+\s*:\s*\d+\s*(?:,|;)' *
> >
> > spaces before colon
> > spaces after colon
> > aligned on bitfield size
> >
> > I'd just as soon make it a --strict check.
>
> I'd be fine with removing the check entirely, or making it --strict;
> however, even for --strict mode I'd suggest applying this patch, as I
> don't think checkpatch should ever prohibit spaces before the ':'.
Maybe something like that patch, but spaces probably
should be prohibited after the : in a bitfield either.
This style is pretty common
u32 dma_address0 :30;
u32 dma_0No_update : 1;
u32 dma_0start : 1;
u32 dma_addr_size :24;
u32 DMA_maxpackets : 8;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Allow spaces before the ':' of a bitfield
2013-10-14 18:04 ` Joe Perches
@ 2013-10-14 20:20 ` Josh Triplett
2013-10-14 20:23 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Josh Triplett @ 2013-10-14 20:20 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Nandini Hanumanthagowda, Greg Kroah-Hartman,
Andy Whitcroft
On Mon, Oct 14, 2013 at 11:04:38AM -0700, Joe Perches wrote:
> On Mon, 2013-10-14 at 11:00 -0700, Josh Triplett wrote:
> > On Mon, Oct 14, 2013 at 10:51:48AM -0700, Joe Perches wrote:
> > > On Mon, 2013-10-14 at 10:32 -0700, Josh Triplett wrote:
> > > > checkpatch warns about spaces both before and after the ':' separating a
> > > > bitfield name from its width. However, many drivers do put space before
> > > > the : to line up the widths, which makes the definition significantly
> > > > more readable; checkpatch should not warn about that. Remove the
> > > > warning for space before the ':' of a bitfield.
> > >
> > > There are a _bunch_ of different styles used for
> > > alignment of bitfields.
> > >
> > > $ grep -rPh --include=*.[ch] \
> > > '(?:unsigned\s+|signed\s+|)(char|short|long|long\s+long|\w+_t|(?:__|)(?:u|s)(?:8|16|32|64))\s+\w+\s*:\s*\d+\s*(?:,|;)' *
> > >
> > > spaces before colon
> > > spaces after colon
> > > aligned on bitfield size
> > >
> > > I'd just as soon make it a --strict check.
> >
> > I'd be fine with removing the check entirely, or making it --strict;
> > however, even for --strict mode I'd suggest applying this patch, as I
> > don't think checkpatch should ever prohibit spaces before the ':'.
>
> Maybe something like that patch, but spaces probably
> should be prohibited after the : in a bitfield either.
>
> This style is pretty common
>
> u32 dma_address0 :30;
> u32 dma_0No_update : 1;
> u32 dma_0start : 1;
> u32 dma_addr_size :24;
> u32 DMA_maxpackets : 8;
I'm fine with just dropping the check entirely; want a patch for that
instead? It's a much simpler patch.
- Josh Triplett
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] checkpatch: Allow spaces before the ':' of a bitfield
2013-10-14 20:20 ` Josh Triplett
@ 2013-10-14 20:23 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2013-10-14 20:23 UTC (permalink / raw)
To: Josh Triplett
Cc: linux-kernel, Nandini Hanumanthagowda, Greg Kroah-Hartman,
Andy Whitcroft
On Mon, 2013-10-14 at 13:20 -0700, Josh Triplett wrote:
> On Mon, Oct 14, 2013 at 11:04:38AM -0700, Joe Perches wrote:
> > On Mon, 2013-10-14 at 11:00 -0700, Josh Triplett wrote:
> > > On Mon, Oct 14, 2013 at 10:51:48AM -0700, Joe Perches wrote:
> > > > On Mon, 2013-10-14 at 10:32 -0700, Josh Triplett wrote:
> > > > > checkpatch warns about spaces both before and after the ':' separating a
> > > > > bitfield name from its width. However, many drivers do put space before
> > > > > the : to line up the widths, which makes the definition significantly
> > > > > more readable; checkpatch should not warn about that. Remove the
> > > > > warning for space before the ':' of a bitfield.
> > > >
> > > > There are a _bunch_ of different styles used for
> > > > alignment of bitfields.
> > > >
> > > > $ grep -rPh --include=*.[ch] \
> > > > '(?:unsigned\s+|signed\s+|)(char|short|long|long\s+long|\w+_t|(?:__|)(?:u|s)(?:8|16|32|64))\s+\w+\s*:\s*\d+\s*(?:,|;)' *
> > > >
> > > > spaces before colon
> > > > spaces after colon
> > > > aligned on bitfield size
> > > >
> > > > I'd just as soon make it a --strict check.
> > >
> > > I'd be fine with removing the check entirely, or making it --strict;
> > > however, even for --strict mode I'd suggest applying this patch, as I
> > > don't think checkpatch should ever prohibit spaces before the ':'.
> >
> > Maybe something like that patch, but spaces probably
> > should be prohibited after the : in a bitfield either.
> >
> > This style is pretty common
> >
> > u32 dma_address0 :30;
> > u32 dma_0No_update : 1;
> > u32 dma_0start : 1;
> > u32 dma_addr_size :24;
> > u32 DMA_maxpackets : 8;
>
> I'm fine with just dropping the check entirely; want a patch for that
> instead? It's a much simpler patch.
Maybe Andy has an opinion? He's the one that
wrote almost all of these SPACING checks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-14 20:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20131013121635.GA29395@nandinih-Inspiron-5521>
[not found] ` <20131013153943.GA27861@kroah.com>
[not found] ` <687af3bd-edc6-40cf-b994-0d0fdb7cd30d@googlegroups.com>
2013-10-14 17:32 ` [PATCH] checkpatch: Allow spaces before the ':' of a bitfield Josh Triplett
2013-10-14 17:51 ` Joe Perches
2013-10-14 18:00 ` Josh Triplett
2013-10-14 18:04 ` Joe Perches
2013-10-14 20:20 ` Josh Triplett
2013-10-14 20:23 ` Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).