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
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?
2015-12-11 17:08 GMT-03:00 Joe Perches <[email protected]>:
> 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
2015-12-11 17:21 GMT-03:00 Geyslan G. Bem <[email protected]>:
> 2015-12-11 17:08 GMT-03:00 Joe Perches <[email protected]>:
>> 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
On Fri, 2015-12-11 at 18:56 -0300, Geyslan G. Bem wrote:
> 2015-12-11 17:21 GMT-03:00 Geyslan G. Bem <[email protected]>:
> > 2015-12-11 17:08 GMT-03:00 Joe Perches <[email protected]>:
> > > 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 = ...;
}
2015-12-11 22:34 GMT-03:00 Joe Perches <[email protected]>:
> On Fri, 2015-12-11 at 18:56 -0300, Geyslan G. Bem wrote:
>> 2015-12-11 17:21 GMT-03:00 Geyslan G. Bem <[email protected]>:
>> > 2015-12-11 17:08 GMT-03:00 Joe Perches <[email protected]>:
>> > > 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