From mboxrd@z Thu Jan 1 00:00:00 1970 From: Darren Hart Date: Sat, 14 Mar 2015 21:06:12 +0000 Subject: Re: [patch 1/2] thinkpad_acpi: signedness bugs getting current_mode Message-Id: <20150314210612.GA26465@fury.dvhart.com> List-Id: References: <20150311093450.GA3564@mwanda> In-Reply-To: <20150311093450.GA3564@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter Cc: Henrique de Moraes Holschuh , Bastien Nocera , ibm-acpi-devel@lists.sourceforge.net, platform-driver-x86@vger.kernel.org, kernel-janitors@vger.kernel.org, Stephen Rothwell , Rafael Wysocki , "H. Peter Anvin" On Wed, Mar 11, 2015 at 12:34:50PM +0300, Dan Carpenter wrote: > 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 > 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? > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 56eaddc..024861d 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -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; > > -- Darren Hart Intel Open Source Technology Center From mboxrd@z Thu Jan 1 00:00:00 1970 From: Darren Hart Subject: Re: [patch 1/2] thinkpad_acpi: signedness bugs getting current_mode Date: Sat, 14 Mar 2015 14:06:12 -0700 Message-ID: <20150314210612.GA26465@fury.dvhart.com> References: <20150311093450.GA3564@mwanda> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from bombadil.infradead.org ([198.137.202.9]:37004 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752043AbbCNVG2 (ORCPT ); Sat, 14 Mar 2015 17:06:28 -0400 Content-Disposition: inline In-Reply-To: <20150311093450.GA3564@mwanda> Sender: platform-driver-x86-owner@vger.kernel.org List-ID: To: Dan Carpenter Cc: Henrique de Moraes Holschuh , Bastien Nocera , ibm-acpi-devel@lists.sourceforge.net, platform-driver-x86@vger.kernel.org, kernel-janitors@vger.kernel.org, Stephen Rothwell , Rafael Wysocki , "H. Peter Anvin" On Wed, Mar 11, 2015 at 12:34:50PM +0300, Dan Carpenter wrote: > 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 > 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? > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index 56eaddc..024861d 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -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; > > -- Darren Hart Intel Open Source Technology Center