* [BUG] checkpatch warning default switch case
@ 2015-12-11 19:34 Geyslan G. Bem
2015-12-11 20:08 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan G. Bem @ 2015-12-11 19:34 UTC (permalink / raw)
To: LKML; +Cc: Peter Senna Tschudin, Joe Perches, Robo Bot
Hello,
Running
scripts/checkpatch.pl -f drivers/usb/host/ehci-sched.c
I got this
...
ERROR: spaces required around that ':' (ctx:VxE)
#496: FILE: drivers/usb/host/ehci-sched.c:496:
+ default:
...
496: default:
ehci_dbg (ehci,
"periodic frame %d bogus type %d\n",
frame, type);
}
It seems a correct use of default switch case. Maybe the test concerns
to ternary conditional.
Let me know.
--
Regards,
Geyslan G. Bem
hackingbits.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [BUG] checkpatch warning default switch case
2015-12-11 19:34 [BUG] checkpatch warning default switch case Geyslan G. Bem
@ 2015-12-11 20:08 ` Joe Perches
2015-12-11 20:21 ` Geyslan G. Bem
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2015-12-11 20:08 UTC (permalink / raw)
To: Geyslan G. Bem, LKML, Andy Whitcroft; +Cc: Peter Senna Tschudin, Robo Bot
On Fri, 2015-12-11 at 16:34 -0300, Geyslan G. Bem wrote:
> Hello,
>
> Running
> scripts/checkpatch.pl -f drivers/usb/host/ehci-sched.c
>
> I got this
> ...
> ERROR: spaces required around that ':' (ctx:VxE)
> #496: FILE: drivers/usb/host/ehci-sched.c:496:
> + default:
> ...
>
>
> 496: default:
> ehci_dbg (ehci,
> "periodic frame %d bogus type %d\n",
> frame, type);
> }
>
> It seems a correct use of default switch case. Maybe the test concerns
> to ternary conditional.
>
> Let me know.
Yes, it's a checkpatch defect.
It's because a c99 style comment is immediately before this line.
switch (foo) {
case 1:
break;
// foo
default:
break;
}
emits this ERROR
I suggest you fix the comment and the message will go away.
Andy? Do you care to fix this defect?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [BUG] checkpatch warning default switch case
2015-12-11 20:08 ` Joe Perches
@ 2015-12-11 20:21 ` Geyslan G. Bem
2015-12-11 21:56 ` Geyslan G. Bem
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan G. Bem @ 2015-12-11 20:21 UTC (permalink / raw)
To: Joe Perches; +Cc: LKML, Andy Whitcroft, Peter Senna Tschudin
2015-12-11 17:08 GMT-03:00 Joe Perches <joe@perches.com>:
> On Fri, 2015-12-11 at 16:34 -0300, Geyslan G. Bem wrote:
>> Hello,
>>
>> Running
>> scripts/checkpatch.pl -f drivers/usb/host/ehci-sched.c
>>
>> I got this
>> ...
>> ERROR: spaces required around that ':' (ctx:VxE)
>> #496: FILE: drivers/usb/host/ehci-sched.c:496:
>> + default:
>> ...
>>
>>
>> 496: default:
>> ehci_dbg (ehci,
>> "periodic frame %d bogus type %d\n",
>> frame, type);
>> }
>>
>> It seems a correct use of default switch case. Maybe the test concerns
>> to ternary conditional.
>>
>> Let me know.
>
> Yes, it's a checkpatch defect.
>
> It's because a c99 style comment is immediately before this line.
>
> switch (foo) {
> case 1:
> break;
> // foo
> default:
> break;
> }
>
> emits this ERROR
>
> I suggest you fix the comment and the message will go away.
Ok, I'm fixing it.
Tks.
>
> Andy? Do you care to fix this defect?
>
--
Regards,
Geyslan G. Bem
hackingbits.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [BUG] checkpatch warning default switch case
2015-12-11 20:21 ` Geyslan G. Bem
@ 2015-12-11 21:56 ` Geyslan G. Bem
2015-12-12 1:34 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan G. Bem @ 2015-12-11 21:56 UTC (permalink / raw)
To: Joe Perches; +Cc: LKML, Andy Whitcroft, Peter Senna Tschudin
2015-12-11 17:21 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
> 2015-12-11 17:08 GMT-03:00 Joe Perches <joe@perches.com>:
>> On Fri, 2015-12-11 at 16:34 -0300, Geyslan G. Bem wrote:
>>> Hello,
>>>
>>> Running
>>> scripts/checkpatch.pl -f drivers/usb/host/ehci-sched.c
>>>
>>> I got this
>>> ...
>>> ERROR: spaces required around that ':' (ctx:VxE)
>>> #496: FILE: drivers/usb/host/ehci-sched.c:496:
>>> + default:
>>> ...
>>>
>>>
>>> 496: default:
>>> ehci_dbg (ehci,
>>> "periodic frame %d bogus type %d\n",
>>> frame, type);
>>> }
>>>
>>> It seems a correct use of default switch case. Maybe the test concerns
>>> to ternary conditional.
>>>
>>> Let me know.
>>
>> Yes, it's a checkpatch defect.
>>
>> It's because a c99 style comment is immediately before this line.
>>
>> switch (foo) {
>> case 1:
>> break;
>> // foo
>> default:
>> break;
>> }
>>
>> emits this ERROR
>>
>> I suggest you fix the comment and the message will go away.
> Ok, I'm fixing it.
>
> Tks.
>>
>> Andy? Do you care to fix this defect?
>>
>
Another false positive:
WARNING: braces {} are not necessary for single statement blocks
#1570: FILE: drivers/usb/host/ehci-sched.c:1570:
+ else {
+ start = (stream->ps.phase << 3) +
stream->ps.phase_uf;
+ }
The code:
if (stream->ps.phase == NO_FRAME) {
int done = 0;
...
reserve_release_iso_bandwidth(ehci, stream, 1);
}
/* New stream is already scheduled; use the upcoming slot */
else {
start = (stream->ps.phase << 3) + stream->ps.phase_uf;
}
There's a C89 comment before the else branch and immediately before
that an empty line. They, I think, are confusing the parser.
>
>
> --
> Regards,
>
> Geyslan G. Bem
> hackingbits.com
--
Regards,
Geyslan G. Bem
hackingbits.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [BUG] checkpatch warning default switch case
2015-12-11 21:56 ` Geyslan G. Bem
@ 2015-12-12 1:34 ` Joe Perches
2015-12-12 1:51 ` Geyslan G. Bem
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2015-12-12 1:34 UTC (permalink / raw)
To: Geyslan G. Bem; +Cc: LKML, Andy Whitcroft, Peter Senna Tschudin
On Fri, 2015-12-11 at 18:56 -0300, Geyslan G. Bem wrote:
> 2015-12-11 17:21 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
> > 2015-12-11 17:08 GMT-03:00 Joe Perches <joe@perches.com>:
> > > On Fri, 2015-12-11 at 16:34 -0300, Geyslan G. Bem wrote:
> > > > Hello,
> > > >
> > > > Running
> > > > scripts/checkpatch.pl -f drivers/usb/host/ehci-sched.c
> > > >
> > > > I got this
> > > > ...
> > > > ERROR: spaces required around that ':' (ctx:VxE)
> > > > #496: FILE: drivers/usb/host/ehci-sched.c:496:
> > > > + default:
> > > > ...
> > > >
> > > >
> > > > 496: default:
> > > > ehci_dbg (ehci,
> > > > "periodic frame %d bogus type
> > > > %d\n",
> > > > frame, type);
> > > > }
> > > >
> > > > It seems a correct use of default switch case. Maybe the test
> > > > concerns
> > > > to ternary conditional.
> > > >
> > > > Let me know.
> > >
> > > Yes, it's a checkpatch defect.
> > >
> > > It's because a c99 style comment is immediately before this line.
> > >
> > > switch (foo) {
> > > case 1:
> > > break;
> > > // foo
> > > default:
> > > break;
> > > }
> > >
> > > emits this ERROR
> > >
> > > I suggest you fix the comment and the message will go away.
> > Ok, I'm fixing it.
> >
> > Tks.
> > >
> > > Andy? Do you care to fix this defect?
> > >
> >
>
> Another false positive:
>
> WARNING: braces {} are not necessary for single statement blocks
> #1570: FILE: drivers/usb/host/ehci-sched.c:1570:
> + else {
> + start = (stream->ps.phase << 3) +
> stream->ps.phase_uf;
> + }
>
> The code:
>
> if (stream->ps.phase == NO_FRAME) {
> int done = 0;
> ...
> reserve_release_iso_bandwidth(ehci, stream, 1);
> }
>
> /* New stream is already scheduled; use the upcoming slot */
> else {
> start = (stream->ps.phase << 3) + stream->ps.phase_uf;
> }
>
> There's a C89 comment before the else branch and immediately before
> that an empty line. They, I think, are confusing the parser.
checkpatch isn't a real parser
Move the comment into the else block
if (...) {
...;
} else {
/* New stream ... */
start = ...;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [BUG] checkpatch warning default switch case
2015-12-12 1:34 ` Joe Perches
@ 2015-12-12 1:51 ` Geyslan G. Bem
0 siblings, 0 replies; 6+ messages in thread
From: Geyslan G. Bem @ 2015-12-12 1:51 UTC (permalink / raw)
To: Joe Perches; +Cc: LKML, Andy Whitcroft, Peter Senna Tschudin
2015-12-11 22:34 GMT-03:00 Joe Perches <joe@perches.com>:
> On Fri, 2015-12-11 at 18:56 -0300, Geyslan G. Bem wrote:
>> 2015-12-11 17:21 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
>> > 2015-12-11 17:08 GMT-03:00 Joe Perches <joe@perches.com>:
>> > > On Fri, 2015-12-11 at 16:34 -0300, Geyslan G. Bem wrote:
>> > > > Hello,
>> > > >
>> > > > Running
>> > > > scripts/checkpatch.pl -f drivers/usb/host/ehci-sched.c
>> > > >
>> > > > I got this
>> > > > ...
>> > > > ERROR: spaces required around that ':' (ctx:VxE)
>> > > > #496: FILE: drivers/usb/host/ehci-sched.c:496:
>> > > > + default:
>> > > > ...
>> > > >
>> > > >
>> > > > 496: default:
>> > > > ehci_dbg (ehci,
>> > > > "periodic frame %d bogus type
>> > > > %d\n",
>> > > > frame, type);
>> > > > }
>> > > >
>> > > > It seems a correct use of default switch case. Maybe the test
>> > > > concerns
>> > > > to ternary conditional.
>> > > >
>> > > > Let me know.
>> > >
>> > > Yes, it's a checkpatch defect.
>> > >
>> > > It's because a c99 style comment is immediately before this line.
>> > >
>> > > switch (foo) {
>> > > case 1:
>> > > break;
>> > > // foo
>> > > default:
>> > > break;
>> > > }
>> > >
>> > > emits this ERROR
>> > >
>> > > I suggest you fix the comment and the message will go away.
>> > Ok, I'm fixing it.
>> >
>> > Tks.
>> > >
>> > > Andy? Do you care to fix this defect?
>> > >
>> >
>>
>> Another false positive:
>>
>> WARNING: braces {} are not necessary for single statement blocks
>> #1570: FILE: drivers/usb/host/ehci-sched.c:1570:
>> + else {
>> + start = (stream->ps.phase << 3) +
>> stream->ps.phase_uf;
>> + }
>>
>> The code:
>>
>> if (stream->ps.phase == NO_FRAME) {
>> int done = 0;
>> ...
>> reserve_release_iso_bandwidth(ehci, stream, 1);
>> }
>>
>> /* New stream is already scheduled; use the upcoming slot */
>> else {
>> start = (stream->ps.phase << 3) + stream->ps.phase_uf;
>> }
>>
>> There's a C89 comment before the else branch and immediately before
>> that an empty line. They, I think, are confusing the parser.
>
> checkpatch isn't a real parser
>
> Move the comment into the else block
>
> if (...) {
> ...;
> } else {
> /* New stream ... */
> start = ...;
> }
>
>
Tks. Patch sent.
--
Regards,
Geyslan G. Bem
hackingbits.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-12 1:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-11 19:34 [BUG] checkpatch warning default switch case Geyslan G. Bem
2015-12-11 20:08 ` Joe Perches
2015-12-11 20:21 ` Geyslan G. Bem
2015-12-11 21:56 ` Geyslan G. Bem
2015-12-12 1:34 ` Joe Perches
2015-12-12 1:51 ` Geyslan G. Bem
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox