On 22 September 2015 at 18:38, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Tue, Sep 15, 2015 at 12:53 PM, Eric Curtin <ericcurtin17@xxxxxxxxx> wrote: >> My first kernel patch, hope I did everything correctly! Instead of calling strlen on every iteration of the for loop, just call it once instead and store in a variable. > > Heh. Ok, that resulted in a rather long email thread. > > Anyway, I'd actually prefer to merge this patch, for two reasons: > > - the "termination calculation is expensive" problem is a real > problem, and while in this case the compiler may be able to notice > that the "strlen()" is constant over the loop and can be hoisted up, > that is not at all necessarily the case most of the time. > > So I actually think patches like this are good things. Not because > this particular code site necessarily matters, but because people who > write code with things like "strlen()" in the terminating condition > need to learn that it's *wrong*. > > - I'd much rather see this kind of trivial patch than the usual > trivial patch that is clearly just "let's run some script on the > kernel and fix up warnings it generates mindlessly". > > In contrast to that, *this* trivial patch was about somebody who > thought about code generation and efficiency. And *that* is the kind > of trivial patches we want to encourage, not necessariyl because this > particular case was so important, but because that's the kind of > people and thinking we want to encourage. > > So quite frankly, I'd just take this directly, but I'd like more of > real changelog. > > But I wonder if Eric is even reading the emails any more ;) > > Linus Hi Linus & Everyone else, I can deal with the patronizing remarks made at the start of the thread, I've been called worse names! But I would encourage people to avoid this behavior especially with newbies, as many may leave the community and never attempt to submit a patch again. It does not leave a great first impression. We so not come out the womb expert kernel developers, you gotta learn how to crawl, before you can walk. Yeah, I'm still reading this email thread and learned lots from it. I'm working on something more meaningful, but it's not going to be ground breaking of course, there is a led on my capslock key on a new machine I won at work that does not switch off properly after it is switched on. I think it is something to do with the LED_CAPSL variable in here: drivers/hid/usbhid/usbkbd.c I'll figure it out, and get it fixed. Decided, I'd do something more meaty. Then I'll start looking at the drivers/staging stuff. I can only look at this stuff for a few hours every second day, with my normal 9 to 5 job being first priority. I can clean up this patch re-submit no problem, in fact I already did and got more feedback, it's subject is "tools: usbip: detach: avoid calling strlen() at each iteration". But I moved on to the keyboard driver because of the feedback I received about wasting peoples time! Regards, Eric -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
