[PATCH] hid-core: Avoid uninitialized buffer access

2015-09-18 Thread Darren Hart
From: Richard Purdie <richard.pur...@linuxfoundation.org>

hid_connect adds various strings to the buffer but they're all
conditional. You can find circumstances where nothing would be written
to it but the kernel will still print the supposedly empty buffer with
printk. This leads to corruption on the console/in the logs.

Ensure buf is initialized to an empty string.

Signed-off-by: Richard Purdie <richard.pur...@linuxfoundation.org>
[dvhart: Initialize string to "" rather than assign buf[0] = NULL;]
Cc: Jiri Kosina <ji...@kernel.org>
Cc: linux-input@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Darren Hart <dvh...@linux.intel.com>
---
 drivers/hid/hid-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 70a11ac..c0fbf4e 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1611,7 +1611,7 @@ int hid_connect(struct hid_device *hdev, unsigned int 
connect_mask)
"Multi-Axis Controller"
};
const char *type, *bus;
-   char buf[64];
+   char buf[64] = "";
unsigned int i;
int len;
int ret;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] use more descriptive KEY_ROTATE_DISPLAY instead of KEY_DIRECTION

2015-03-06 Thread Darren Hart
On Thu, Mar 05, 2015 at 11:56:00PM -0800, Dmitry Torokhov wrote:
 On Thu, Mar 05, 2015 at 12:31:10AM +0100, Stefan Brüns wrote:
  Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de
 
 This should go through HID and platform trees unless I hear otherwise.

I'll defer to Jiri on this. Jiri, please feel free to pull the
drivers/platform/x86 changes through your tree.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ibm-acpi-devel] [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

2015-03-06 Thread Darren Hart
On Tue, Mar 03, 2015 at 03:39:54PM -0300, Henrique de Moraes Holschuh wrote:
 On Tue, Mar 3, 2015, at 13:52, Darren Hart wrote:
  Henrique, I believe I may have overstepped with thinkpad-acpi and dealt
  with it like the other drivers in platform/drivers/x86, when instead I 
  should
  have been leaving it to you. My apologies, it was not intentional.
 
 No harm done!  And I did appreciate the extra help :-)
 
  Do you typically send pull-requests for the thinkpad-acpi driver directly 
  to Linus?
 
 thinkpad-acpi is part of the platform-x86 subsystem, so IMHO it should
 continue to be merged through your tree if you are okay with that.
 
 I'd really appreciate if the typical patch approval flux would go
 through me first.  However, I certainly don't mind if you merge patches
 directly should I be unresponsible for some reason, or when they are
 part of the usual tree-wide spot fixes for some pattern/anti-pattern,
 etc.
 
  I would be more than happy to basically ignore anything to thinkpad-acpi 
  until
  after you have provided a review. I can also roll up pull requests from
  you if you prefer to integrate into the platform-drivers-x86 tree as the 
  path to
  Linus.
 
 Well, if it is fine with you, I'd suggest we continue with the process
 used in this patchset, where I will reply to patches in this ML with a
 reviewed-by or acked-by before you merge them.
 
 If you want to change that at some point, just drop me a note and I will
 start pushing patches to you either through this ML or through signed
 pull requests, whichever you prefer.

Great. Things are documented correctly in MAINTAINERS then and we can continue
as before, only I will be more diligent making sure you are Cc'd and have
responded to any thinkpad-acpi.c changes. I assume two weeks is sufficient, but
may nag you after a week.

Thanks!

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ibm-acpi-devel] [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

2015-03-03 Thread Darren Hart
On Fri, Feb 27, 2015 at 08:16:19AM -0300, Henrique de Moraes Holschuh wrote:
 On Fri, Feb 27, 2015, at 08:05, Henrique de Moraes Holschuh wrote:
  On Thu, Feb 26, 2015, at 03:18, Darren Hart wrote:
   On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
Rather than checking on each suspend and resume whether the laptop
has an adaptive keyboard, check when the driver is initialised.
   
   Bastien, am I awaiting another version of this from you to address
   comments from
   Henrique?
   
   Henrique, when you're satisfied, please provide a Reviewed-by for the
   series.
  
  I usually provide a signed-off-by, as I am the thinkpad-acpi driver
  maintainer... reviewed-by is implied in that case.
 
 Or an Acked-by, for that matter.  Which is what I'd use for this series,
 since there is no reason to gatekeep it and it is being sent to you
 directly.
 

Henrique, I believe I may have overstepped with thinkpad-acpi and dealt with it
like the other drivers in platform/drivers/x86, when instead I should have been
leaving it to you. My apologies, it was not intentional.

Do you typically send pull-requests for the thinkpad-acpi driver
directly to Linus? Geeze, I see that the tree listed in MAINTAINERS is not even
mine. Ugh, my sincere apologies.

I would be more than happy to basically ignore anything to thinkpad-acpi until
after you have provided a review. I can also roll up pull requests from you if
you prefer to integrate into the platform-drivers-x86 tree as the path to Linus.

How do you want to handle your driver?

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] thinkpad_acpi: Add support for more adaptive kbd buttons

2015-03-03 Thread Darren Hart
On Mon, Mar 02, 2015 at 02:17:01PM -0300, Henrique de Moraes Holschuh wrote:
 On Mon, Mar 2, 2015, at 10:45, Bastien Nocera wrote:
  This commit adds new elements to the ThinkPad keymaps, and
  will send key events for keys for which an input.h declaration
  exists.
  
  Signed-off-by: Bastien Nocera had...@hadess.net
 
 Reviewed-by: Henrique de Moraes Holschuh h...@hmh.eng.br
 Acked-by: Henrique de Moraes Holschuh h...@hmh.eng.br

As I understand it, and how I interpret these tags, Reviewed-by is a superset of
Acked-by. Acked implies acceptance of the general idea, Reviewed implies a
careful code review. No need for both.

I'm queueing these for testing in the pdx86 tree because we've been discussing
that approach. However, I've asked Henrique to let me know how he prefers to
handle this driver.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

2015-02-25 Thread Darren Hart
On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
 Rather than checking on each suspend and resume whether the laptop
 has an adaptive keyboard, check when the driver is initialised.

Bastien, am I awaiting another version of this from you to address comments from
Henrique?

Henrique, when you're satisfied, please provide a Reviewed-by for the series.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 11/20] power_supply: Change ownership from driver to core

2015-02-25 Thread Darren Hart
] (seq_read) from [c00e53dc] 
  (__vfs_read+0x18/0x4c)
  [   55.979188] [c00e53dc] (__vfs_read) from [c00e548c] 
  (vfs_read+0x7c/0x100)
  [   55.986304] [c00e548c] (vfs_read) from [c00e5550] 
  (SyS_read+0x40/0x8c)
  [   55.993164] [c00e5550] (SyS_read) from [c000f1a0] 
  (ret_fast_syscall+0x0/0x48)
  [   56.000626] Code: bad PC value
  [   56.011652] ---[ end trace 7b64343fbdae8ef1 ]---
  
  Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
  Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
  
  [for the nvec part]
  Reviewed-by: Marc Dietrich marvi...@gmx.de
  ---
   arch/x86/platform/olpc/olpc-xo1-sci.c |   4 +-
   arch/x86/platform/olpc/olpc-xo15-sci.c|   4 +-
   drivers/acpi/ac.c |  32 +++---
   drivers/acpi/battery.c|  55 +-
   drivers/acpi/sbs.c|  68 +++-
   drivers/hid/hid-input.c   |  51 +
   drivers/hid/hid-sony.c|  43 
   drivers/hid/hid-wiimote-modules.c |  41 +++
   drivers/hid/hid-wiimote.h |   3 +-
   drivers/hid/wacom.h   |   8 +-
   drivers/hid/wacom_sys.c   |  71 ++--
   drivers/platform/x86/compal-laptop.c  |  29 +++--
   drivers/power/[...]   |  lots of changes
   drivers/staging/nvec/nvec_power.c |  29 +++--
   include/linux/hid.h   |   6 +-
   include/linux/mfd/abx500/ux500_chargalg.h |  11 +-
   include/linux/mfd/rt5033.h|   2 +-
   include/linux/mfd/wm8350/supply.h |   6 +-
   include/linux/power/charger-manager.h |   3 +-
   include/linux/power_supply.h  |  39 ---
   80 files changed, 2172 insertions(+), 1756 deletions(-)
 
 I would like to merge this via the power supply subsystem. Some of
 the files being patched are not under my maintainence, though. It
 would be nice if I get an Acked-By from their maintainers.
 
 As far as I can see this patch is currently missing feedback from:
 
 arch/x86/platform/olpc: x86 Maintainers
 drivers/acpi: Rafael J. Wysocki, Len Brown
 drivers/hid: Jiri Kosina
 drivers/platform/x86: Darren Hart

For compal-laptop.c:

Acked-by: Darren Hart dvh...@linux.intel.com

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] thinkpad_acpi: Add support for more adaptive kbd buttons

2015-02-19 Thread Darren Hart
 +
 + TPACPI_HOTKEY_MAP_LEN -
 + ADAPTIVE_KEY_OFFSET) {
 + pr_info(Unhandled adaptive keyboard key: 0x%x\n,
 + scancode);
 + return false;
 + }
 + keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY + 
 ADAPTIVE_KEY_OFFSET];

Please be consistent with your line wrapping. The if block above wraps early and
is a rather difficult to scan, while the keycode index goes well beyond 80
chars.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6] thinkpad_acpi: Remember adaptive kbd presence

2015-02-19 Thread Darren Hart
On Wed, Feb 18, 2015 at 09:53:28PM +0100, Bastien Nocera wrote:
 Rather than checking on each suspend and resume whether the laptop
 has an adaptive keyboard, check when the driver is initialised.

Reasonable.

 
 Signed-off-by: Bastien Nocera had...@hadess.net
 ---
  drivers/platform/x86/thinkpad_acpi.c | 38 
 ++--
  1 file changed, 19 insertions(+), 19 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index c3d11fa..80db3ce 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
...
 @@ -3226,6 +3227,13 @@ static int __init hotkey_init(struct ibm_init_struct 
 *iibm)
   if (!tp_features.hotkey)
   return 1;
  
 + /* does it have an adaptive keyboard, like
 +  * the Lenovo Carbon X1 2014 (2nd gen) */

Please follow CodingStyle here:

/*
 * Sentence formatting.
 * Second line.
 */

The above is a question, just state what you're doing:

/*
 * Check if we have an adaptive keyboard, like on the
 * Lenovo Carbon X1 2014 (2nd Gen).
 */

 + if (acpi_evalf(hkey_handle, hkeyv, MHKV, qd)) {
 + if ((hkeyv  8) == 2)
 + tp_features.has_adaptive_kbd = true;
 + }
 +
   quirks = tpacpi_check_quirks(tpacpi_hotkey_qtable,
ARRAY_SIZE(tpacpi_hotkey_qtable));
  
 @@ -3834,28 +3842,22 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 
 event)
  
  static void hotkey_suspend(void)
  {
 - int hkeyv;
 -
   /* Do these on suspend, we get the events on early resume! */
   hotkey_wakeup_reason = TP_ACPI_WAKEUP_NONE;
   hotkey_autosleep_ack = 0;
  
   /* save previous mode of adaptive keyboard of X1 Carbon */
 - if (acpi_evalf(hkey_handle, hkeyv, MHKV, qd)) {
 - if ((hkeyv  8) == 2) {
 - if (!acpi_evalf(hkey_handle,
 - adaptive_keyboard_prev_mode,
 - GTRW, dd, 0)) {
 - pr_err(Cannot read adaptive keyboard mode.\n);
 - }
 + if (tp_features.has_adaptive_kbd) {
 + if (!acpi_evalf(hkey_handle,
 + adaptive_keyboard_prev_mode,
 + GTRW, dd, 0)) {
 + pr_err(Cannot read adaptive keyboard mode.\n);
   }
   }
  }
  
  static void hotkey_resume(void)
  {
 - int hkeyv;
 -
   tpacpi_disable_brightness_delay();
  
   if (hotkey_status_set(true)  0 ||
 @@ -3870,14 +3872,12 @@ static void hotkey_resume(void)
   hotkey_poll_setup_safe(false);
  
   /* restore previous mode of adapive keyboard of X1 Carbon */
 - if (acpi_evalf(hkey_handle, hkeyv, MHKV, qd)) {
 - if ((hkeyv  8) == 2) {
 - if (!acpi_evalf(hkey_handle,
 - NULL,
 - STRW, vd,
 - adaptive_keyboard_prev_mode)) {
 - pr_err(Cannot set adaptive keyboard mode.\n);
 - }
 + if (tp_features.has_adaptive_kbd) {
 + if (!acpi_evalf(hkey_handle,
 + NULL,
 + STRW, vd,
 + adaptive_keyboard_prev_mode)) {

You've saved a level of indentation, please group the arguments into fewer
lines, should be two instead of four.

Otherwise, looks good.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/6] thinkpad_acpi: Factor out get/set adaptive kbd mode

2015-02-19 Thread Darren Hart
On Wed, Feb 18, 2015 at 09:53:35PM +0100, Bastien Nocera wrote:

Please provide a commit message. There is always something to say beyond what is
in the subject. In this case, I suggest the motivation and justification for the
change.

While I appreciate the abstraction, it makes the code at the call site easier to
read, note that you added more code than you removed.

So, please provide a justificaiton.

Under no circumstances will I accept a patch without a commit message body.

 Signed-off-by: Bastien Nocera had...@hadess.net
 ---
  drivers/platform/x86/thinkpad_acpi.c | 61 
 ++--
  1 file changed, 38 insertions(+), 23 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index 80db3ce..a6dd017 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -3480,6 +3480,32 @@ const int adaptive_keyboard_modes[] = {
  static bool adaptive_keyboard_mode_is_saved;
  static int adaptive_keyboard_prev_mode;
  
 +static int adaptive_keyboard_get_mode(void)
 +{
 + u32 mode = 0;

acpi_evalf second argument takes an int and this function returns int. Is
there a reason to use u32 for mode?

...

 @@ -3509,39 +3535,28 @@ static bool 
 adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
   new_mode = adaptive_keyboard_prev_mode;
   adaptive_keyboard_mode_is_saved = false;
   } else {
 - if (!acpi_evalf(
 - hkey_handle, current_mode,
 - GTRW, dd, 0)) {
 - pr_err(Cannot read adaptive keyboard mode\n);
 + current_mode = adaptive_keyboard_get_mode();
 + if (current_mode  0)
   return false;
 - } else {
 - new_mode = adaptive_keyboard_get_next_mode(
 - current_mode);
 - }
 + new_mode = adaptive_keyboard_get_next_mode(
 + current_mode);

This now fits on one line I believe.

...

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/6] thinkpad_acpi: Add adaptive_kbd_mode sysfs attr

2015-02-19 Thread Darren Hart
On Wed, Feb 18, 2015 at 09:53:44PM +0100, Bastien Nocera wrote:

Commit message please.

 Signed-off-by: Bastien Nocera had...@hadess.net
 ---
  drivers/platform/x86/thinkpad_acpi.c | 71 
 +++-
  1 file changed, 62 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index a6dd017..562d958 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c

...

 +static struct device_attribute dev_attr_adaptive_kbd_mode =
 + __ATTR(adaptive_kbd_mode, S_IWUSR | S_IRUGO,
 + adaptive_kbd_mode_show, adaptive_kbd_mode_store);
 +

Please use DEVICE_ATTR_RW() macros for new sysfs files.

I'd very much like to see a cleanup of the driver to use these as well.

Henrique, your thoughts / preference?

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 02/20] power_supply: Move run-time configuration to separate structure

2015-02-06 Thread Darren Hart
On Fri, Jan 30, 2015 at 03:47:40PM +0100, Krzysztof Kozlowski wrote:
 Add new structure 'power_supply_config' for holding run-time
 initialization data like of_node, supplies and private driver data.
 
 The power_supply_register() function is changed so all power supply
 drivers need updating.
 
 When registering the power supply this new 'power_supply_config' should be
 used instead of directly initializing 'struct power_supply'. This allows
 changing the ownership of power_supply structure from driver to the
 power supply core in next patches.
 
 When a driver does not use of_node or supplies then it should use NULL
 as config. If driver uses of_node or supplies then it should allocate
 config on stack and initialize it with proper values.
 
 Signed-off-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 Reviewed-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com

For drivers/platform/x86/compal-laptop.c

Reviewed-by: Darren Hart dvh...@linux.intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html