[PATCH] input: fix coding style issues in input.c

[PATCH] input: fix coding style issues in input.c

[Nick Simonov]

This is a patch to the input.c file that fixes
up warning found by checkpatch.pl tool

Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>
---
 drivers/input/input.c | 52 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 9785546..e18fdae 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * The input core
  *
@@ -252,7 +253,8 @@ static int input_handle_abs_event(struct input_dev *dev,
 	}
 
 	/* Flush pending "slot" event */
-	if (is_mt_event && mt && mt->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
+	if (is_mt_event && mt && mt->slot !=
+		input_abs_get_val(dev, ABS_MT_SLOT)) {
 		input_abs_set_val(dev, ABS_MT_SLOT, mt->slot);
 		return INPUT_PASS_TO_HANDLERS | INPUT_SLOT;
 	}
@@ -969,7 +971,8 @@ bool input_match_device_id(const struct input_dev *dev,
 }
 EXPORT_SYMBOL(input_match_device_id);
 
-static const struct input_device_id *input_match_device(struct input_handler *handler,
+static
+const struct input_device_id *input_match_device(struct input_handler *handler,
 							struct input_dev *dev)
 {
 	const struct input_device_id *id;
@@ -984,7 +987,8 @@ static const struct input_device_id *input_match_device(struct input_handler *ha
 	return NULL;
 }
 
-static int input_attach_handler(struct input_dev *dev, struct input_handler *handler)
+static
+int input_attach_handler(struct input_dev *dev, struct input_handler *handler)
 {
 	const struct input_device_id *id;
 	int error;
@@ -1010,6 +1014,7 @@ static int input_bits_to_string(char *buf, int buf_size,
 
 	if (in_compat_syscall()) {
 		u32 dword = bits >> 32;
+
 		if (dword || !skip_empty)
 			len += snprintf(buf, buf_size, "%x ", dword);
 
@@ -1132,7 +1137,10 @@ static int input_devices_seq_show(struct seq_file *seq, void *v)
 	struct input_handle *handle;
 
 	seq_printf(seq, "I: Bus=%04x Vendor=%04x Product=%04x Version=%04x\n",
-		   dev->id.bustype, dev->id.vendor, dev->id.product, dev->id.version);
+		   dev->id.bustype,
+		   dev->id.vendor,
+		   dev->id.product,
+		   dev->id.version);
 
 	seq_printf(seq, "N: Name=\"%s\"\n", dev->name ? dev->name : "");
 	seq_printf(seq, "P: Phys=%s\n", dev->phys ? dev->phys : "");
@@ -1221,9 +1229,10 @@ static void *input_handlers_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 
 static int input_handlers_seq_show(struct seq_file *seq, void *v)
 {
-	struct input_handler *handler = container_of(v, struct input_handler, node);
+	struct input_handler *handler;
 	union input_seq_state *state = (union input_seq_state *)&seq->private;
 
+	handler = container_of(v, struct input_handler, node);
 	seq_printf(seq, "N: Number=%u Name=%s", state->pos, handler->name);
 	if (handler->filter)
 		seq_puts(seq, " (filter)");
@@ -1302,7 +1311,7 @@ static ssize_t input_dev_show_##name(struct device *dev,		\
 	return scnprintf(buf, PAGE_SIZE, "%s\n",			\
 			 input_dev->name ? input_dev->name : "");	\
 }									\
-static DEVICE_ATTR(name, S_IRUGO, input_dev_show_##name, NULL)
+static DEVICE_ATTR(name, 0444, input_dev_show_##name, NULL)
 
 INPUT_DEV_STRING_ATTR_SHOW(name);
 INPUT_DEV_STRING_ATTR_SHOW(phys);
@@ -1317,7 +1326,8 @@ static int input_print_modalias_bits(char *buf, int size,
 	len += snprintf(buf, max(size, 0), "%c", name);
 	for (i = min_bit; i < max_bit; i++)
 		if (bm[BIT_WORD(i)] & BIT_MASK(i))
-			len += snprintf(buf + len, max(size - len, 0), "%X,", i);
+			len += snprintf(buf + len, max(size - len, 0),
+							"%X,", i);
 	return len;
 }
 
@@ -1327,9 +1337,9 @@ static int input_print_modalias(char *buf, int size, struct input_dev *id,
 	int len;
 
 	len = snprintf(buf, max(size, 0),
-		       "input:b%04Xv%04Xp%04Xe%04X-",
-		       id->id.bustype, id->id.vendor,
-		       id->id.product, id->id.version);
+				"input:b%04Xv%04Xp%04Xe%04X-",
+				id->id.bustype, id->id.vendor,
+				id->id.product, id->id.version);
 
 	len += input_print_modalias_bits(buf + len, size - len,
 				'e', id->evbit, 0, EV_MAX);
@@ -1367,7 +1377,7 @@ static ssize_t input_dev_show_modalias(struct device *dev,
 
 	return min_t(int, len, PAGE_SIZE);
 }
-static DEVICE_ATTR(modalias, S_IRUGO, input_dev_show_modalias, NULL);
+static DEVICE_ATTR(modalias, 0444, input_dev_show_modalias, NULL);
 
 static int input_print_bitmap(char *buf, int buf_size, unsigned long *bitmap,
 			      int max, int add_cr);
@@ -1381,7 +1391,7 @@ static ssize_t input_dev_show_properties(struct device *dev,
 				     INPUT_PROP_MAX, true);
 	return min_t(int, len, PAGE_SIZE);
 }
-static DEVICE_ATTR(properties, S_IRUGO, input_dev_show_properties, NULL);
+static DEVICE_ATTR(properties, 0444, input_dev_show_properties, NULL);
 
 static struct attribute *input_dev_attrs[] = {
 	&dev_attr_name.attr,
@@ -1404,7 +1414,7 @@ static ssize_t input_dev_show_id_##name(struct device *dev,		\
 	struct input_dev *input_dev = to_input_dev(dev);		\
 	return scnprintf(buf, PAGE_SIZE, "%04x\n", input_dev->id.name);	\
 }									\
-static DEVICE_ATTR(name, S_IRUGO, input_dev_show_id_##name, NULL)
+static DEVICE_ATTR(name, 0444, input_dev_show_id_##name, NULL)
 
 INPUT_DEV_ID_ATTR(bustype);
 INPUT_DEV_ID_ATTR(vendor);
@@ -1437,7 +1447,8 @@ static int input_print_bitmap(char *buf, int buf_size, unsigned long *bitmap,
 		if (len) {
 			skip_empty = false;
 			if (i > 0)
-				len += snprintf(buf + len, max(buf_size - len, 0), " ");
+				len += snprintf(buf + len,
+						max(buf_size - len, 0), " ");
 		}
 	}
 
@@ -1464,7 +1475,7 @@ static ssize_t input_dev_show_cap_##bm(struct device *dev,		\
 				     true);				\
 	return min_t(int, len, PAGE_SIZE);				\
 }									\
-static DEVICE_ATTR(bm, S_IRUGO, input_dev_show_cap_##bm, NULL)
+static DEVICE_ATTR(bm, 0444, input_dev_show_cap_##bm, NULL)
 
 INPUT_DEV_CAP_ATTR(EV, ev);
 INPUT_DEV_CAP_ATTR(KEY, key);
@@ -1519,7 +1530,9 @@ static void input_dev_release(struct device *device)
  * device bitfields.
  */
 static int input_add_uevent_bm_var(struct kobj_uevent_env *env,
-				   const char *name, unsigned long *bitmap, int max)
+				   const char *name,
+				   unsigned long *bitmap,
+				   int max)
 {
 	int len;
 
@@ -1899,7 +1912,8 @@ EXPORT_SYMBOL(input_free_device);
  * In addition to setting up corresponding bit in appropriate capability
  * bitmap the function also adjusts dev->evbit.
  */
-void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int code)
+void input_set_capability(struct input_dev *dev,
+					 unsigned int type, unsigned int code)
 {
 	switch (type) {
 	case EV_KEY:
@@ -1943,8 +1957,8 @@ void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int
 		break;
 
 	default:
-		pr_err("input_set_capability: unknown type %u (code %u)\n",
-		       type, code);
+		pr_err("%s: unknown type %u (code %u)\n",
+			   __func__, type, code);
 		dump_stack();
 		return;
 	}
-- 
2.7.4

Re: [PATCH] input: fix coding style issues in input.c

[Dmitry Torokhov]

Hi NIck,

On Wed, May 09, 2018 at 05:07:14PM +0300, Nick Simonov wrote:
> This is a patch to the input.c file that fixes
> up warning found by checkpatch.pl tool
> 
> Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>
> ---
>  drivers/input/input.c | 52 ++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 33 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 9785546..e18fdae 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * The input core
>   *
> @@ -252,7 +253,8 @@ static int input_handle_abs_event(struct input_dev *dev,
>  	}
>  
>  	/* Flush pending "slot" event */
> -	if (is_mt_event && mt && mt->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
> +	if (is_mt_event && mt && mt->slot !=
> +		input_abs_get_val(dev, ABS_MT_SLOT)) {
>  		input_abs_set_val(dev, ABS_MT_SLOT, mt->slot);

So now it is not immediately clear what is part of condition and what is
part of body.

I am sorry to say, but with most of these changes the cure is worse than
the disease. If you were fixing the code and adjusted the affected lines
so they are under 80 columns limit that would be one thing, but just
reformatting for the sake of it is not really helpful.

Thanks.

-- 
Dmitry

Re: [PATCH] input: fix coding style issues in input.c

[Joe Perches]

On Wed, 2018-05-09 at 17:33 -0700, Dmitry Torokhov wrote:
> Hi Nick,
> On Wed, May 09, 2018 at 05:07:14PM +0300, Nick Simonov wrote:
> > This is a patch to the input.c file that fixes
> > up warning found by checkpatch.pl tool
> > 
> > Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>
> > ---
> >  drivers/input/input.c | 52 ++++++++++++++++++++++++++++++++-------------------
> >  1 file changed, 33 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > index 9785546..e18fdae 100644
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> >   * The input core
> >   *
> > @@ -252,7 +253,8 @@ static int input_handle_abs_event(struct input_dev *dev,
> >  	}
> >  
> >  	/* Flush pending "slot" event */
> > -	if (is_mt_event && mt && mt->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
> > +	if (is_mt_event && mt && mt->slot !=
> > +		input_abs_get_val(dev, ABS_MT_SLOT)) {
> >  		input_abs_set_val(dev, ABS_MT_SLOT, mt->slot);
> 
> So now it is not immediately clear what is part of condition and what is
> part of body.
> 
> I am sorry to say, but with most of these changes the cure is worse than
> the disease. If you were fixing the code and adjusted the affected lines
> so they are under 80 columns limit that would be one thing, but just
> reformatting for the sake of it is not really helpful.

Completely true.

btw: it might be faster to avoid the
	mt->slot != input_abs_get_val(dev, ABS_MT_SLOT)
test and simply always do the write.

So maybe just:

	if (is_mt_event && mt)
		input_abs_set_val(dev, ABS_MT_SLOT, mt->slot);

And to make it easier to grep the definition of input_abs_set_val
and the other get/sets it might be better to add something like:
---
 include/linux/input.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/input.h b/include/linux/input.h
index 7c7516eb7d76..243cf4e3006a 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -459,6 +459,14 @@ static inline void input_abs_set_##_suffix(struct input_dev *dev,	\
 		dev->absinfo[axis]._item = val;				\
 }
 
+/* static inline definitions of:
+ *	input_abs_get_val/input_abs_set_val
+ *	input_abs_get_min/input_abs_set_min
+ *	input_abs_get_max/input_abs_set_max
+ *	input_abs_get_fuzz/input_abs_set_fuzz
+ *	input_abs_get_flat/input_abs_set_flat
+ *	input_abs_get_res/input_abs_set_res
+ */
 INPUT_GENERATE_ABS_ACCESSORS(val, value)
 INPUT_GENERATE_ABS_ACCESSORS(min, minimum)
 INPUT_GENERATE_ABS_ACCESSORS(max, maximum)

Re: [PATCH] input: fix coding style issues in input.c

[Nick Simonov]

[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]

 Wed, May 09, 2018 at 05:33:13PM -0700, Dmitry Torokhov wrote:
> Hi NIck,
> 
> On Wed, May 09, 2018 at 05:07:14PM +0300, Nick Simonov wrote:
> > This is a patch to the input.c file that fixes
> > up warning found by checkpatch.pl tool
> > 
> > Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>
> > ---
> >  drivers/input/input.c | 52 ++++++++++++++++++++++++++++++++-------------------
> >  1 file changed, 33 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > index 9785546..e18fdae 100644
> > --- a/drivers/input/input.c
> > +++ b/drivers/input/input.c
> > @@ -1,3 +1,4 @@
> > +// SPDX-License-Identifier: GPL-2.0
> >  /*
> >   * The input core
> >   *
> > @@ -252,7 +253,8 @@ static int input_handle_abs_event(struct input_dev *dev,
> >  	}
> >  
> >  	/* Flush pending "slot" event */
> > -	if (is_mt_event && mt && mt->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
> > +	if (is_mt_event && mt && mt->slot !=
> > +		input_abs_get_val(dev, ABS_MT_SLOT)) {
> >  		input_abs_set_val(dev, ABS_MT_SLOT, mt->slot);
> 
> So now it is not immediately clear what is part of condition and what is
> part of body.
> 
> I am sorry to say, but with most of these changes the cure is worse than
> the disease. If you were fixing the code and adjusted the affected lines
> so they are under 80 columns limit that would be one thing, but just
> reformatting for the sake of it is not really helpful.
> 
> Thanks.
> 
> -- 
> Dmitry

Dmitry thanks for your comment. I deleted all my changes except one
and prepare a new patch for it. 

In function input_set_capability when it go through default statment 
it is use hard coded function name "input_set_capability" in pr_err() call.
I replace it using "%s" __func__ instead.

[-- Attachment #2: 0001-input-replace-hard-coded-string-with-__func__-in-pr_.patch --]
[-- Type: text/plain, Size: 950 bytes --]

>From 2aef27ca4896b8d9e64fd1417965793acfba3653 Mon Sep 17 00:00:00 2001
From: Nick Simonov <nicksimonovv@gmail.com>
Date: Sat, 12 May 2018 01:24:47 +0300
Subject: [PATCH] input: replace hard coded string with __func__ in pr_err()

Change hardcoded string "input_set_capability"
in pr_err() function call, replace it with
"%s" __func__ instead.

Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>
---
 drivers/input/input.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 9785546..6365c19 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1943,8 +1943,7 @@ void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int
 		break;
 
 	default:
-		pr_err("input_set_capability: unknown type %u (code %u)\n",
-		       type, code);
+		pr_err("%s: unknown type %u (code %u)\n", __func__, type, code);
 		dump_stack();
 		return;
 	}
-- 
2.7.4

Re: [PATCH] input: fix coding style issues in input.c

[Dmitry Torokhov]

On Sat, May 12, 2018 at 02:01:20AM +0300, Nick Simonov wrote:
>  Wed, May 09, 2018 at 05:33:13PM -0700, Dmitry Torokhov wrote:
> > Hi NIck,
> > 
> > On Wed, May 09, 2018 at 05:07:14PM +0300, Nick Simonov wrote:
> > > This is a patch to the input.c file that fixes
> > > up warning found by checkpatch.pl tool
> > > 
> > > Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>
> > > ---
> > >  drivers/input/input.c | 52 ++++++++++++++++++++++++++++++++-------------------
> > >  1 file changed, 33 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/input/input.c b/drivers/input/input.c
> > > index 9785546..e18fdae 100644
> > > --- a/drivers/input/input.c
> > > +++ b/drivers/input/input.c
> > > @@ -1,3 +1,4 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > >  /*
> > >   * The input core
> > >   *
> > > @@ -252,7 +253,8 @@ static int input_handle_abs_event(struct input_dev *dev,
> > >  	}
> > >  
> > >  	/* Flush pending "slot" event */
> > > -	if (is_mt_event && mt && mt->slot != input_abs_get_val(dev, ABS_MT_SLOT)) {
> > > +	if (is_mt_event && mt && mt->slot !=
> > > +		input_abs_get_val(dev, ABS_MT_SLOT)) {
> > >  		input_abs_set_val(dev, ABS_MT_SLOT, mt->slot);
> > 
> > So now it is not immediately clear what is part of condition and what is
> > part of body.
> > 
> > I am sorry to say, but with most of these changes the cure is worse than
> > the disease. If you were fixing the code and adjusted the affected lines
> > so they are under 80 columns limit that would be one thing, but just
> > reformatting for the sake of it is not really helpful.
> > 
> > Thanks.
> > 
> > -- 
> > Dmitry
> 
> Dmitry thanks for your comment. I deleted all my changes except one
> and prepare a new patch for it. 
> 
> In function input_set_capability when it go through default statment 
> it is use hard coded function name "input_set_capability" in pr_err() call.
> I replace it using "%s" __func__ instead.

> From 2aef27ca4896b8d9e64fd1417965793acfba3653 Mon Sep 17 00:00:00 2001
> From: Nick Simonov <nicksimonovv@gmail.com>
> Date: Sat, 12 May 2018 01:24:47 +0300
> Subject: [PATCH] input: replace hard coded string with __func__ in pr_err()
> 
> Change hardcoded string "input_set_capability"
> in pr_err() function call, replace it with
> "%s" __func__ instead.
> 
> Signed-off-by: Nick Simonov <nicksimonovv@gmail.com>

Applied, thank you.

> ---
>  drivers/input/input.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 9785546..6365c19 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1943,8 +1943,7 @@ void input_set_capability(struct input_dev *dev, unsigned int type, unsigned int
>  		break;
>  
>  	default:
> -		pr_err("input_set_capability: unknown type %u (code %u)\n",
> -		       type, code);
> +		pr_err("%s: unknown type %u (code %u)\n", __func__, type, code);
>  		dump_stack();
>  		return;
>  	}
> -- 
> 2.7.4
> 


-- 
Dmitry