diff mbox

Input: don't push static constants on stack for %*ph

Message ID 1378291309-13869-1-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive)
State Rejected
Headers show

Commit Message

Andy Shevchenko Sept. 4, 2013, 10:41 a.m. UTC
There is no need to pass constants via stack. The width may be explicitly
specified in the format.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/input/mouse/hgpk.c               | 7 +++----
 drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
 2 files changed, 5 insertions(+), 6 deletions(-)

Comments

Andy Shevchenko Nov. 14, 2013, 3:07 p.m. UTC | #1
On Wed, 2013-09-04 at 13:41 +0300, Andy Shevchenko wrote:
> There is no need to pass constants via stack. The width may be explicitly
> specified in the format.

Any comments on this so far?

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/input/mouse/hgpk.c               | 7 +++----
>  drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/hgpk.c b/drivers/input/mouse/hgpk.c
> index 62be888..27909a1 100644
> --- a/drivers/input/mouse/hgpk.c
> +++ b/drivers/input/mouse/hgpk.c
> @@ -333,9 +333,8 @@ static bool hgpk_is_byte_valid(struct psmouse *psmouse, unsigned char *packet)
>  	}
>  
>  	if (!valid)
> -		psmouse_dbg(psmouse,
> -			    "bad data, mode %d (%d) %*ph\n",
> -			    priv->mode, pktcnt, 6, psmouse->packet);
> +		psmouse_dbg(psmouse, "bad data, mode %d (%d) %6ph\n",
> +			    priv->mode, pktcnt, psmouse->packet);
>  
>  	return valid;
>  }
> @@ -1027,7 +1026,7 @@ static enum hgpk_model_t hgpk_get_model(struct psmouse *psmouse)
>  		return -EIO;
>  	}
>  
> -	psmouse_dbg(psmouse, "ID: %*ph\n", 3, param);
> +	psmouse_dbg(psmouse, "ID: %3ph\n", param);
>  
>  	/* HGPK signature: 0x67, 0x00, 0x<model> */
>  	if (param[0] != 0x67 || param[1] != 0x00)
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 59aa240..dd44ebf 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -331,8 +331,8 @@ static bool mxt_object_writable(unsigned int type)
>  static void mxt_dump_message(struct device *dev,
>  			     struct mxt_message *message)
>  {
> -	dev_dbg(dev, "reportid: %u\tmessage: %*ph\n",
> -		message->reportid, 7, message->message);
> +	dev_dbg(dev, "reportid: %u\tmessage: %7ph\n",
> +		message->reportid, message->message);
>  }
>  
>  static int mxt_check_bootloader(struct i2c_client *client,
Andy Shevchenko Dec. 10, 2015, 1:46 p.m. UTC | #2
On Wed, 2013-09-04 at 13:41 +0300, Andy Shevchenko wrote:
> There is no need to pass constants via stack. The width may be
> explicitly
> specified in the format.

Dmitry, any comments on this?

> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/input/mouse/hgpk.c               | 7 +++----
>  drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/hgpk.c b/drivers/input/mouse/hgpk.c
> index 62be888..27909a1 100644
> --- a/drivers/input/mouse/hgpk.c
> +++ b/drivers/input/mouse/hgpk.c
> @@ -333,9 +333,8 @@ static bool hgpk_is_byte_valid(struct psmouse
> *psmouse, unsigned char *packet)
>  	}
>  
>  	if (!valid)
> -		psmouse_dbg(psmouse,
> -			    "bad data, mode %d (%d) %*ph\n",
> -			    priv->mode, pktcnt, 6, psmouse->packet);
> +		psmouse_dbg(psmouse, "bad data, mode %d (%d)
> %6ph\n",
> +			    priv->mode, pktcnt, psmouse->packet);
>  
>  	return valid;
>  }
> @@ -1027,7 +1026,7 @@ static enum hgpk_model_t hgpk_get_model(struct
> psmouse *psmouse)
>  		return -EIO;
>  	}
>  
> -	psmouse_dbg(psmouse, "ID: %*ph\n", 3, param);
> +	psmouse_dbg(psmouse, "ID: %3ph\n", param);
>  
>  	/* HGPK signature: 0x67, 0x00, 0x<model> */
>  	if (param[0] != 0x67 || param[1] != 0x00)
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 59aa240..dd44ebf 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -331,8 +331,8 @@ static bool mxt_object_writable(unsigned int
> type)
>  static void mxt_dump_message(struct device *dev,
>  			     struct mxt_message *message)
>  {
> -	dev_dbg(dev, "reportid: %u\tmessage: %*ph\n",
> -		message->reportid, 7, message->message);
> +	dev_dbg(dev, "reportid: %u\tmessage: %7ph\n",
> +		message->reportid, message->message);
>  }
>  
>  static int mxt_check_bootloader(struct i2c_client *client,
Dmitry Torokhov Dec. 15, 2015, 7:31 a.m. UTC | #3
On Thu, Dec 10, 2015 at 5:46 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Wed, 2013-09-04 at 13:41 +0300, Andy Shevchenko wrote:
>> There is no need to pass constants via stack. The width may be
>> explicitly
>> specified in the format.
>
> Dmitry, any comments on this?

My personal preference is to have length as a parameter instead of
encoding in format string as it allows using symbolic constants and
expressions (even though in these particular cases we use numeric
constants). The code is not in critical paths so we can live with
extra arguments.

Thanks.

>
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/input/mouse/hgpk.c               | 7 +++----
>>  drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
>>  2 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/mouse/hgpk.c b/drivers/input/mouse/hgpk.c
>> index 62be888..27909a1 100644
>> --- a/drivers/input/mouse/hgpk.c
>> +++ b/drivers/input/mouse/hgpk.c
>> @@ -333,9 +333,8 @@ static bool hgpk_is_byte_valid(struct psmouse
>> *psmouse, unsigned char *packet)
>>       }
>>
>>       if (!valid)
>> -             psmouse_dbg(psmouse,
>> -                         "bad data, mode %d (%d) %*ph\n",
>> -                         priv->mode, pktcnt, 6, psmouse->packet);
>> +             psmouse_dbg(psmouse, "bad data, mode %d (%d)
>> %6ph\n",
>> +                         priv->mode, pktcnt, psmouse->packet);
>>
>>       return valid;
>>  }
>> @@ -1027,7 +1026,7 @@ static enum hgpk_model_t hgpk_get_model(struct
>> psmouse *psmouse)
>>               return -EIO;
>>       }
>>
>> -     psmouse_dbg(psmouse, "ID: %*ph\n", 3, param);
>> +     psmouse_dbg(psmouse, "ID: %3ph\n", param);
>>
>>       /* HGPK signature: 0x67, 0x00, 0x<model> */
>>       if (param[0] != 0x67 || param[1] != 0x00)
>> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
>> b/drivers/input/touchscreen/atmel_mxt_ts.c
>> index 59aa240..dd44ebf 100644
>> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
>> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
>> @@ -331,8 +331,8 @@ static bool mxt_object_writable(unsigned int
>> type)
>>  static void mxt_dump_message(struct device *dev,
>>                            struct mxt_message *message)
>>  {
>> -     dev_dbg(dev, "reportid: %u\tmessage: %*ph\n",
>> -             message->reportid, 7, message->message);
>> +     dev_dbg(dev, "reportid: %u\tmessage: %7ph\n",
>> +             message->reportid, message->message);
>>  }
>>
>>  static int mxt_check_bootloader(struct i2c_client *client,
>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
>
Nick Dyer Dec. 17, 2015, 5:16 p.m. UTC | #4
> On Wed, 2013-09-04 at 13:41 +0300, Andy Shevchenko wrote:
>> There is no need to pass constants via stack. The width may be
>> explicitly
>> specified in the format.
> 
> Dmitry, any comments on this?

Hi Andy-

Just to weigh in on the atmel_mxt_ts part - it's made moot by:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/touchscreen/atmel_mxt_ts.c?id=5f3f9bc2b1f3a09e732df

regards

Nick

> 
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/input/mouse/hgpk.c               | 7 +++----
>>  drivers/input/touchscreen/atmel_mxt_ts.c | 4 ++--
>>  2 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/input/mouse/hgpk.c b/drivers/input/mouse/hgpk.c
>> index 62be888..27909a1 100644
>> --- a/drivers/input/mouse/hgpk.c
>> +++ b/drivers/input/mouse/hgpk.c
>> @@ -333,9 +333,8 @@ static bool hgpk_is_byte_valid(struct psmouse
>> *psmouse, unsigned char *packet)
>>  	}
>>  
>>  	if (!valid)
>> -		psmouse_dbg(psmouse,
>> -			    "bad data, mode %d (%d) %*ph\n",
>> -			    priv->mode, pktcnt, 6, psmouse->packet);
>> +		psmouse_dbg(psmouse, "bad data, mode %d (%d)
>> %6ph\n",
>> +			    priv->mode, pktcnt, psmouse->packet);
>>  
>>  	return valid;
>>  }
>> @@ -1027,7 +1026,7 @@ static enum hgpk_model_t hgpk_get_model(struct
>> psmouse *psmouse)
>>  		return -EIO;
>>  	}
>>  
>> -	psmouse_dbg(psmouse, "ID: %*ph\n", 3, param);
>> +	psmouse_dbg(psmouse, "ID: %3ph\n", param);
>>  
>>  	/* HGPK signature: 0x67, 0x00, 0x<model> */
>>  	if (param[0] != 0x67 || param[1] != 0x00)
>> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
>> b/drivers/input/touchscreen/atmel_mxt_ts.c
>> index 59aa240..dd44ebf 100644
>> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
>> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
>> @@ -331,8 +331,8 @@ static bool mxt_object_writable(unsigned int
>> type)
>>  static void mxt_dump_message(struct device *dev,
>>  			     struct mxt_message *message)
>>  {
>> -	dev_dbg(dev, "reportid: %u\tmessage: %*ph\n",
>> -		message->reportid, 7, message->message);
>> +	dev_dbg(dev, "reportid: %u\tmessage: %7ph\n",
>> +		message->reportid, message->message);
>>  }
>>  
>>  static int mxt_check_bootloader(struct i2c_client *client,
> 

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

Patch

diff --git a/drivers/input/mouse/hgpk.c b/drivers/input/mouse/hgpk.c
index 62be888..27909a1 100644
--- a/drivers/input/mouse/hgpk.c
+++ b/drivers/input/mouse/hgpk.c
@@ -333,9 +333,8 @@  static bool hgpk_is_byte_valid(struct psmouse *psmouse, unsigned char *packet)
 	}
 
 	if (!valid)
-		psmouse_dbg(psmouse,
-			    "bad data, mode %d (%d) %*ph\n",
-			    priv->mode, pktcnt, 6, psmouse->packet);
+		psmouse_dbg(psmouse, "bad data, mode %d (%d) %6ph\n",
+			    priv->mode, pktcnt, psmouse->packet);
 
 	return valid;
 }
@@ -1027,7 +1026,7 @@  static enum hgpk_model_t hgpk_get_model(struct psmouse *psmouse)
 		return -EIO;
 	}
 
-	psmouse_dbg(psmouse, "ID: %*ph\n", 3, param);
+	psmouse_dbg(psmouse, "ID: %3ph\n", param);
 
 	/* HGPK signature: 0x67, 0x00, 0x<model> */
 	if (param[0] != 0x67 || param[1] != 0x00)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 59aa240..dd44ebf 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -331,8 +331,8 @@  static bool mxt_object_writable(unsigned int type)
 static void mxt_dump_message(struct device *dev,
 			     struct mxt_message *message)
 {
-	dev_dbg(dev, "reportid: %u\tmessage: %*ph\n",
-		message->reportid, 7, message->message);
+	dev_dbg(dev, "reportid: %u\tmessage: %7ph\n",
+		message->reportid, message->message);
 }
 
 static int mxt_check_bootloader(struct i2c_client *client,