Linus Torvalds Shows His New Polite Side While Pointing Out Bad Kernel Code
When Linus Torvalds announced last month that he would be taking a temporary leave of absence to work on his empathy and interpersonal skills as well as the adoption of a Linux kernel Code of Conduct, some Internet commenters thought this would lead Linus to being less strict about code quality and his standards for accepting new code to the mainline tree. Fortunately, he's shown already for the new Linux 4.20~5.0 cycle he isn't relaxing his standards but is communicating better when it comes to bringing up coding issues.
Today he took issue with the HID pull request and its introduction of the BigBen game controller driver that was introduced: the developer enabled this new driver by default. Linus Torvalds has always frowned upon random new drivers being enabled by default in the kernel configuration driver. Today he still voiced his opinion over this driver's default "Y" build configuration, but did so in a more professional manner than he has done in the past:
There was also another situation today testing Torvalds' patience when it came to a kernel oops, with the reformed Torvalds writing:
That's compared to Linus previously in such incidents calling out code as "untested crap" and often profanity laced messages.
So far it looks like Linus' brief retreat is paying off with still addressing code quality issues -- and not blatantly accepting new code into the kernel as some feared -- but in doing so in a professional manner compared to his past manner of exclaiming himself over capitalized sentences and profanity that at time put him at odds with some in the Linux kernel community.
Today he took issue with the HID pull request and its introduction of the BigBen game controller driver that was introduced: the developer enabled this new driver by default. Linus Torvalds has always frowned upon random new drivers being enabled by default in the kernel configuration driver. Today he still voiced his opinion over this driver's default "Y" build configuration, but did so in a more professional manner than he has done in the past:
We do *not* enable new random drivers by default. And we most *definitely* don't do it when they are odd-ball ones that most people have never heard of.In comparison, here's his post from last November over a similar issue:
Yet the new "BigBen Interactive" driver that was added this merge window did exactly that.
Just don't do it.
Yes, yes, every developer always thinks that _their_ driver is so special and so magically important that it should be enabled by default. But no. When we have thousands of drivers, we don't randomly pick one new driver to be enabled by default just because some developer thinks it is special. It's not.
So the
default !EXPERT
was completely wrong in commit 256a90ed9e46 ("HID: hid-bigbenff: driver for BigBen Interactive PS3OFMINIPAD gamepad"). Please don't do things like this.
Linus
You add new drivers and then default them to "on".
THAT IS COMPLETELY UNACCEPTABLE.
I don't know why I have to say this every single merge window, but let's do it one more time:
As a developer, you think _your_ driver or feature is the most important thing ever, and you have the hardware.
AND ALMOST NOBODY ELSE CARES.
Read it and weep. Unless your hardware is completely ubiquitous, it damn well should not default to being defaulted everybody elses config.
...
But something like CONFIG_DELL_SMBIOS sure as hell does not merit being default on. Not even if you have enabled WMI.
EVERY SINGLE "default" line that got added by this branch was wrong.
Stop doing this. It's a serious violation of peoples expectations. When I do "make oldconfig", I don't want some new random hardware support.
There was also another situation today testing Torvalds' patience when it came to a kernel oops, with the reformed Torvalds writing:
On my laptop I'm getting a kernel page fault with the current git tree, and I'm tentatively blaming commit
9ee3e06610fd ("HID: i2c-hid: override HID descriptors for certain devices")
but that's simply because it's the only thing that seems to touch this particular area in this merge window.
The oops looks like this:
...
which is why I suspect that new i2c_hid_get_dmi_hid_report_desc_override() code.
I *think* the problem is that the i2c_hid_dmi_desc_override_table[] isn't terminated by a NULL entry, and I will test that next.
What makes me *very* unhappy about this is that if I'm right, I think it means that code was literally not tested at all by anybody who didn't have one of the entries in that list.
That's compared to Linus previously in such incidents calling out code as "untested crap" and often profanity laced messages.
So far it looks like Linus' brief retreat is paying off with still addressing code quality issues -- and not blatantly accepting new code into the kernel as some feared -- but in doing so in a professional manner compared to his past manner of exclaiming himself over capitalized sentences and profanity that at time put him at odds with some in the Linux kernel community.
61 Comments