thinkpad_acpi: signedness bugs getting current_mode
Subsystems (2)
PATCH 1/2 thinkpad_acpi: signedness bugs getting current_mode
This needs to be signed for the error handling to work. Valid modes are small positive integers.
drivers/platform/x86/thinkpad_acpi.c +2 -2
@@ -2938,7 +2938,7 @@ static ssize_t adaptive_kbd_mode_show(struct device *dev,struct device_attribute *attr, char *buf) { - u32 current_mode; + int current_mode; current_mode =adaptive_keyboard_get_mode ();if (current_mode <0 )
@@ -3621,7 +3621,7 @@ static int adaptive_keyboard_get_next_mode(int mode)static bool adaptive_keyboard_hotkey_notify_hotkey (unsigned int scancode) { -u32 current_mode =0 ; +int current_mode =0 ;int new_mode =0 ;int keycode;
Quoted 5 lines · Dan Carpenter
This needs to be signed for the error handling to work. Valid modes are small positive integers.
Fixes b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfsattr') Signed-off-by: Dan Carpenter <[email protected]>
Quoted 30 lines · Dan Carpenter
This needs to be signed for the error handling to work. Valid modes are small positive integers.
Fixes b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')Signed-off-by Dan Carpenter <[email protected]>drivers/platform/x86/thinkpad_acpi.c +1
-1@@ -2938,7 +2938,7 @@ static ssize_t adaptive_kbd_mode_show(struct device *dev, struct device_attribute *attr, char *buf) { -u32 current_mode; +int current_mode; current_mode =adaptive_keyboard_get_mode ();drivers/platform/x86/thinkpad_acpi.c +1
-1if (current_mode <0 )@@ -3621,7 +3621,7 @@ static int adaptive_keyboard_get_next_mode(int mode) static bool adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode) { -u32 current_mode =0 ; +int current_mode =0 ;int new_mode =0 ;
Quoted 2 lines
int keycode;
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Darren Hart
AppliedQuoted 6 lines · Dan Carpenter
This needs to be signed for the error handling to work. Valid modes are small positive integers.
Fixes b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')Signed-off-by Dan Carpenter <[email protected]>
Queued, thanks Dan!
Signature · 2 lines
Darren Hart Intel Open Source Technology Center
Quoted 6 lines · Dan Carpenter
This needs to be signed for the error handling to work. Valid modes are small positive integers.
Fixes b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')Signed-off-by Dan Carpenter <[email protected]>
Question for HPA, Rafael, and Stephen,
I recall discussion at Kernel Summit 2014 about not rebasing or merging patches when sending to Linus, that he'd prefer to see the history. I recall Stephen mentioning something similar for linux-next.
That said, I've seen varying behavior among maintainers with respect to fixes like this one from Dan. This patch fixes a patch that currently only exists in my for-next and Stephen's linux-next trees.
What is the preference. Do I just queue it up to for-next as is (this is what I've done for now), or do I roll it into the referred patch causing the error and credit Dan with the fixup?
Left to my own devices I would prefer not to introduce bugs into the kernel history if I can help it. That said, I don't want to make extra work for Stephen or Linus.
What's the prefered best practice here?
Quoted 23 lines
drivers/platform/x86/thinkpad_acpi.c +2
-2@@ -2938,7 +2938,7 @@ static ssize_t adaptive_kbd_mode_show(struct device *dev,struct device_attribute *attr, char *buf) { - u32 current_mode; + int current_mode; current_mode =adaptive_keyboard_get_mode ();if (current_mode <0 )@@ -3621,7 +3621,7 @@ static int adaptive_keyboard_get_next_mode(int mode)static bool adaptive_keyboard_hotkey_notify_hotkey (unsigned int scancode) { -u32 current_mode =0 ; +int current_mode =0 ;int new_mode =0 ;int keycode;
Signature · 2 lines
Darren Hart Intel Open Source Technology Center
Hi Darren,
Quoted 15 lines · On Sat, 14 Mar 2015 14:06:12 -0700 Darren Hart <[email protected]> wrote:
Quoted 6 lines · Dan Carpenter
This needs to be signed for the error handling to work. Valid modes are small positive integers.
Fixes b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')Signed-off-by Dan Carpenter <[email protected]>Question for HPA, Rafael, and Stephen,
I recall discussion at Kernel Summit 2014 about not rebasing or merging patches when sending to Linus, that he'd prefer to see the history. I recall Stephen mentioning something similar for linux-next.
Quoted 7 lines
That said, I've seen varying behavior among maintainers with respect to fixes like this one from Dan. This patch fixes a patch that currently only exists in my for-next and Stephen's linux-next trees.
What is the preference. Do I just queue it up to for-next as is (this is what I've done for now), or do I roll it into the referred patch causing the error and credit Dan with the fixup?
The only answer, is "it depends" :-) If it will cause problems for people doing bisects, or if it is just the last one of two commits on the top of your tree, it seems like it is OK to just squash the fix into the original patch (with attribution as you say). Otherwise, I would prefer that trees just progress with no rebasing.
Quoted 3 lines
Left to my own devices I would prefer not to introduce bugs into the kernel history if I can help it. That said, I don't want to make extra work for Stephen or Linus.
It is very unusual for a small rebase to cause me any more work (unless the files involved and modified by other trees). And for Linus, well all he sees is the tree that you submit to him.
The people you will inconvenience more by rebasing are the developers who write patches that are based on your tree. If you rebase under them, they may have to rebase and fix up the patches they have already tested and had reviewed before they can then submit them to you (hopefully before you rebase again).
Dave Miller seems to get on quite well in the networking tree with no rebasing at all ...
Signature · 2 lines
Cheers, Stephen Rothwell [email protected]
Hi Darren,
Quoted 6 lines · On Sun, 15 Mar 2015 13:46:12 +1100 Stephen Rothwell <[email protected]>…
The people you will inconvenience more by rebasing are the developers who write patches that are based on your tree. If you rebase under them, they may have to rebase and fix up the patches they have already tested and had reviewed before they can then submit them to you (hopefully before you rebase again).
This, of course, only applied to published trees (which includes anything in linux-next) - what you do in your own (logically private) development tree is your own business.
Signature · 2 lines
Cheers, Stephen Rothwell [email protected]
Quoted 10 lines · Stephen Rothwell
Hi Darren,
Quoted 6 lines · On Sun, 15 Mar 2015 13:46:12 +1100 Stephen Rothwell <[email protected]>…
The people you will inconvenience more by rebasing are the developers who write patches that are based on your tree. If you rebase under them, they may have to rebase and fix up the patches they have already tested and had reviewed before they can then submit them to you (hopefully before you rebase again).
Quoted 3 lines
This, of course, only applied to published trees (which includes anything in linux-next) - what you do in your own (logically private) development tree is your own business.
Thanks for the guidance Stephen. Unless a more compelling argument comes up, I'll stick to this going forward.
Signature · 2 lines
Darren Hart Intel Open Source Technology Center
Quoted 5 lines · Dan Carpenter
This needs to be signed for the error handling to work. Valid modes are small positive integers.
Fixes b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')Signed-off-by Dan Carpenter <[email protected]>
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh