From: Valdis.Kletnieks@vt.edu (Valdis.Kletnieks at vt.edu)
To: kernelnewbies@lists.kernelnewbies.org
Subject: [PATCH] usb: Fix switch statement in ohci-tmio.c
Date: Thu, 14 Aug 2014 16:51:46 -0400 [thread overview]
Message-ID: <17204.1408049506@turing-police.cc.vt.edu> (raw)
In-Reply-To: Your message of "Thu, 14 Aug 2014 14:18:40 -0400." <CAPDOMVhSBRJC-Wai+QTTzpU5BDv3Frk-H81rtgMEAbSayHsj2g@mail.gmail.com>
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
next prev parent reply other threads:[~2014-08-14 20:51 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-14 18:05 [PATCH] usb: Fix switch statement in ohci-tmio.c Nicholas Krause
2014-08-14 18:06 ` Nick Krause
2014-08-14 18:13 ` Kristofer Hallin
2014-08-14 18:18 ` Nick Krause
2014-08-14 18:36 ` Lucas Tanure
2014-08-14 20:51 ` Valdis.Kletnieks at vt.edu [this message]
2014-08-14 21:00 ` Nick Krause
2014-08-14 21:03 ` Mandeep Sandhu
2014-08-14 21:13 ` Nick Krause
2014-08-14 21:38 ` Valdis.Kletnieks at vt.edu
2014-08-14 22:12 ` Nick Krause
2014-08-14 23:46 ` Nick Krause
2014-08-15 2:16 ` Greg Freemyer
2014-08-15 2:29 ` Nick Krause
2014-08-15 2:42 ` Nick Krause
2014-08-15 3:37 ` Nick Krause
2014-08-15 3:42 ` Jaret Flores
2014-08-15 3:43 ` Nick Krause
2014-08-15 4:21 ` Nick Krause
2014-08-14 19:04 ` Tobias Boege
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=17204.1408049506@turing-police.cc.vt.edu \
--to=valdis.kletnieks@vt.edu \
--cc=kernelnewbies@lists.kernelnewbies.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).