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

Valdis.Kletnieks at vt.edu Valdis.Kletnieks at vt.edu
Thu Aug 14 16:51:46 EDT 2014


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*.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140814/3021b06f/attachment.bin 


More information about the Kernelnewbies mailing list