[PATCH] usb: Fix switch statement in ohci-tmio.c

Nick Krause xerofoify at gmail.com
Thu Aug 14 17:00:58 EDT 2014


On Thu, Aug 14, 2014 at 4:51 PM,  <Valdis.Kletnieks at vt.edu> wrote:
> On Thu, 14 Aug 2014 14:18:40 -0400, Nick Krause said:
>
>> >>>                 case 3:
>> >>>                         pm |= CCR_PM_USBPW3;
>> >>> +                       break;
>> >>>                 case 2:
>> >>>                         pm |= CCR_PM_USBPW2;
>> >>> +                       break;
>> >>>                 case 1:
>> >>>                         pm |= CCR_PM_USBPW1;
>> >>> +                       break;
>
>> I understand that too but this patch works and builds I have tested it
>> completely.
>
> No, you haven't.  Far from it.
>
> If you were testing it "completely", you would have found an actual Toshiba
> Mobile or other hardware that uses this driver, and had logs to show how it
> used to misbehave due to this bug, and that it worked correctly now.
>
> And you would have noticed, and explained, why the #defines for
> CCR_PM_USBPW2 and CCR_PM_USBPW3 are the same value.  In fact, that's
> why the compiler whined, *not* because of the missing break; code.  So
> sticking in breaks and not even mentioning the #defines shows that you
> did ABSOLUTELY ZERO to actually understand what the compiler was telling you.
>
> Note that there's perfectly valid reasons for fall-through in a C switch -
> see Tom Duff for a classic example (yes, Dennis Ritchie said it was valid C):
>
> http://www.lysator.liu.se/c/duffs-device.html
>
> The #defines could possibly be intentionally that way if the chip is weird and
> uses 2 pins to control 3 ports in a non-intuitive way - and if they're
> active-low bits even the lack of break; after the default: warning case could
> be proper, disabling all ports if an invalid one is specified.
>
> And there's no indication you even *bothered* to check that possibility - which
> would mean applying your patch would break a properly written driver.
>
> And even worse, there's no indication you actually understood what the
> compile warning was telling you.
>
>> Look I got off on the wrong start and I am starting to improve my repo but seems
>
> If you think you're improving your rep with these poor patches, you're delusional.
>
>> impossible if people are just going to forget about my good patches.
>
> We'll discuss that when you actually submit one that isn't a steaming
> pile of dingo's kidneys.
>
> Do yourself a favor - try to resist the temptation to post a patch for at
> least 30 to 60 days, *no matter how correct you think it is*.


Very well then I will not do any more patches for the next month and
only read the list.
Nick



More information about the Kernelnewbies mailing list