[PATCH v1] ipmi: use %*ph to print small buffer

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v1] ipmi: use %*ph to print small buffer
@ 2019-10-11 14:52 Andy Shevchenko
  2019-10-11 14:58 ` Joe Perches
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-10-11 14:52 UTC (permalink / raw)
  To: Corey Minyard, openipmi-developer, linux-kernel
  Cc: Andy Shevchenko, Andy Shevchenko

From: Andy Shevchenko <andy.shevchenko@gmail.com>

Use %*ph format to print small buffer as hex string.

The change is safe since the specifier can handle up to 64 bytes and taking
into account the buffer size of 100 bytes on stack the function has never been
used to dump more than 32 bytes. Note, this also avoids potential buffer
overflow if the length of the input buffer is bigger.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 2aab80e19ae0..d0cefd95fa57 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -48,14 +48,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
 static void ipmi_debug_msg(const char *title, unsigned char *data,
 			   unsigned int len)
 {
-	int i, pos;
-	char buf[100];
-
-	pos = snprintf(buf, sizeof(buf), "%s: ", title);
-	for (i = 0; i < len; i++)
-		pos += snprintf(buf + pos, sizeof(buf) - pos,
-				" %2.2x", data[i]);
-	pr_debug("%s\n", buf);
+	pr_debug("%s: %*ph\n", title, len, buf);
 }
 #else
 static void ipmi_debug_msg(const char *title, unsigned char *data,
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] ipmi: use %*ph to print small buffer
  2019-10-11 14:52 [PATCH v1] ipmi: use %*ph to print small buffer Andy Shevchenko
@ 2019-10-11 14:58 ` Joe Perches
  2019-10-11 15:12   ` Andy Shevchenko
  0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2019-10-11 14:58 UTC (permalink / raw)
  To: Andy Shevchenko, Corey Minyard, openipmi-developer, linux-kernel
  Cc: Andy Shevchenko

On Fri, 2019-10-11 at 17:52 +0300, Andy Shevchenko wrote:
> Use %*ph format to print small buffer as hex string.
> 
> The change is safe since the specifier can handle up to 64 bytes and taking
> into account the buffer size of 100 bytes on stack the function has never been
> used to dump more than 32 bytes. Note, this also avoids potential buffer
> overflow if the length of the input buffer is bigger.
[]
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
[]
> @@ -48,14 +48,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
>  static void ipmi_debug_msg(const char *title, unsigned char *data,
>  			   unsigned int len)
>  {
> -	int i, pos;
> -	char buf[100];
> -
> -	pos = snprintf(buf, sizeof(buf), "%s: ", title);
> -	for (i = 0; i < len; i++)
> -		pos += snprintf(buf + pos, sizeof(buf) - pos,
> -				" %2.2x", data[i]);
> -	pr_debug("%s\n", buf);
> +	pr_debug("%s: %*ph\n", title, len, buf);
>  }
>  #else
>  static void ipmi_debug_msg(const char *title, unsigned char *data,

Now you might as well remove the #ifdef DEBUG above this
and the empty function in the #else too.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] ipmi: use %*ph to print small buffer
  2019-10-11 14:58 ` Joe Perches
@ 2019-10-11 15:12   ` Andy Shevchenko
  2019-10-11 15:18     ` Joe Perches
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-10-11 15:12 UTC (permalink / raw)
  To: Joe Perches; +Cc: Corey Minyard, openipmi-developer, linux-kernel

On Fri, Oct 11, 2019 at 07:58:14AM -0700, Joe Perches wrote:
> On Fri, 2019-10-11 at 17:52 +0300, Andy Shevchenko wrote:
> > Use %*ph format to print small buffer as hex string.
> > 
> > The change is safe since the specifier can handle up to 64 bytes and taking
> > into account the buffer size of 100 bytes on stack the function has never been
> > used to dump more than 32 bytes. Note, this also avoids potential buffer
> > overflow if the length of the input buffer is bigger.
> []
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> []
> > @@ -48,14 +48,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
> >  static void ipmi_debug_msg(const char *title, unsigned char *data,
> >  			   unsigned int len)
> >  {
> > -	int i, pos;
> > -	char buf[100];
> > -
> > -	pos = snprintf(buf, sizeof(buf), "%s: ", title);
> > -	for (i = 0; i < len; i++)
> > -		pos += snprintf(buf + pos, sizeof(buf) - pos,
> > -				" %2.2x", data[i]);
> > -	pr_debug("%s\n", buf);
> > +	pr_debug("%s: %*ph\n", title, len, buf);
> >  }
> >  #else
> >  static void ipmi_debug_msg(const char *title, unsigned char *data,
> 
> Now you might as well remove the #ifdef DEBUG above this
> and the empty function in the #else too.

It's up to maintainer.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] ipmi: use %*ph to print small buffer
  2019-10-11 15:12   ` Andy Shevchenko
@ 2019-10-11 15:18     ` Joe Perches
  2019-10-11 15:36       ` Andy Shevchenko
  2019-10-11 15:46       ` [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph Joe Perches
  0 siblings, 2 replies; 10+ messages in thread
From: Joe Perches @ 2019-10-11 15:18 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Corey Minyard, openipmi-developer, linux-kernel

On Fri, 2019-10-11 at 18:12 +0300, Andy Shevchenko wrote:
> On Fri, Oct 11, 2019 at 07:58:14AM -0700, Joe Perches wrote:
> > On Fri, 2019-10-11 at 17:52 +0300, Andy Shevchenko wrote:
> > > Use %*ph format to print small buffer as hex string.
> > > 
> > > The change is safe since the specifier can handle up to 64 bytes and taking
> > > into account the buffer size of 100 bytes on stack the function has never been
> > > used to dump more than 32 bytes. Note, this also avoids potential buffer
> > > overflow if the length of the input buffer is bigger.
> > []
> > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> > []
> > > @@ -48,14 +48,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
> > >  static void ipmi_debug_msg(const char *title, unsigned char *data,
> > >  			   unsigned int len)
> > >  {
> > > -	int i, pos;
> > > -	char buf[100];
> > > -
> > > -	pos = snprintf(buf, sizeof(buf), "%s: ", title);
> > > -	for (i = 0; i < len; i++)
> > > -		pos += snprintf(buf + pos, sizeof(buf) - pos,
> > > -				" %2.2x", data[i]);
> > > -	pr_debug("%s\n", buf);
> > > +	pr_debug("%s: %*ph\n", title, len, buf);
> > >  }
> > >  #else
> > >  static void ipmi_debug_msg(const char *title, unsigned char *data,
> > 
> > Now you might as well remove the #ifdef DEBUG above this
> > and the empty function in the #else too.
> 
> It's up to maintainer.

That's like suggesting any function with a single pr_debug
should have another duplicative empty function without.

Using code like the below is not good form as it's prone
to defects when the arguments in one block is changed but
not the other.

Also the first form doesn't work with dynamic debug.

#ifdef DEBUG
void debug_print(...)
{
	pr_debug(...);
}
#else
void debug_print(...)
{
}
#endif




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1] ipmi: use %*ph to print small buffer
  2019-10-11 15:18     ` Joe Perches
@ 2019-10-11 15:36       ` Andy Shevchenko
  2019-10-11 15:46       ` [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph Joe Perches
  1 sibling, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2019-10-11 15:36 UTC (permalink / raw)
  To: Joe Perches; +Cc: Corey Minyard, openipmi-developer, linux-kernel

On Fri, Oct 11, 2019 at 08:18:41AM -0700, Joe Perches wrote:
> On Fri, 2019-10-11 at 18:12 +0300, Andy Shevchenko wrote:
> > On Fri, Oct 11, 2019 at 07:58:14AM -0700, Joe Perches wrote:
> > > On Fri, 2019-10-11 at 17:52 +0300, Andy Shevchenko wrote:

> > > >  static void ipmi_debug_msg(const char *title, unsigned char *data,
...
> > > > +	pr_debug("%s: %*ph\n", title, len, buf);
...
> > > >  #else
> > > >  static void ipmi_debug_msg(const char *title, unsigned char *data,

> > > Now you might as well remove the #ifdef DEBUG above this
> > > and the empty function in the #else too.
> > 
> > It's up to maintainer.
> 
> That's like suggesting any function with a single pr_debug
> should have another duplicative empty function without.
> 
> Using code like the below is not good form as it's prone
> to defects when the arguments in one block is changed but
> not the other.
> 
> Also the first form doesn't work with dynamic debug.

I'm surprised to see my name in To:. I guess you intended to explain this to
Corey. I'm fine with either, since I have no idea what is in the IPMI going on.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph
  2019-10-11 15:18     ` Joe Perches
  2019-10-11 15:36       ` Andy Shevchenko
@ 2019-10-11 15:46       ` Joe Perches
  2019-10-11 16:05         ` Andy Shevchenko
                           ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Joe Perches @ 2019-10-11 15:46 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Corey Minyard, openipmi-developer, linux-kernel

Using %*ph format to print small buffers as hex string reduces
overall object size and allows the removal of the two variants
of ipmi_debug_msg.

This also removes unnecessary duplicate colons from output when
enabled by #DEBUG or newly possible CONFIG_DYNAMIC_DEBUG.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 29 ++++++-----------------------
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 2aab80e19ae0..1b1f6a7dc17d 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -44,25 +44,6 @@ static void need_waiter(struct ipmi_smi *intf);
 static int handle_one_recv_msg(struct ipmi_smi *intf,
 			       struct ipmi_smi_msg *msg);
 
-#ifdef DEBUG
-static void ipmi_debug_msg(const char *title, unsigned char *data,
-			   unsigned int len)
-{
-	int i, pos;
-	char buf[100];
-
-	pos = snprintf(buf, sizeof(buf), "%s: ", title);
-	for (i = 0; i < len; i++)
-		pos += snprintf(buf + pos, sizeof(buf) - pos,
-				" %2.2x", data[i]);
-	pr_debug("%s\n", buf);
-}
-#else
-static void ipmi_debug_msg(const char *title, unsigned char *data,
-			   unsigned int len)
-{ }
-#endif
-
 static bool initialized;
 static bool drvregistered;
 
@@ -2267,7 +2248,8 @@ static int i_ipmi_request(struct ipmi_user     *user,
 		ipmi_free_smi_msg(smi_msg);
 		ipmi_free_recv_msg(recv_msg);
 	} else {
-		ipmi_debug_msg("Send", smi_msg->data, smi_msg->data_size);
+		pr_debug("%s: %*ph\n",
+			 "Send", smi_msg->data, smi_msg->data_size);
 
 		smi_send(intf, intf->handlers, smi_msg, priority);
 	}
@@ -3730,7 +3712,8 @@ static int handle_ipmb_get_msg_cmd(struct ipmi_smi *intf,
 		msg->data[10] = ipmb_checksum(&msg->data[6], 4);
 		msg->data_size = 11;
 
-		ipmi_debug_msg("Invalid command:", msg->data, msg->data_size);
+		pr_debug("%s: %*ph\n",
+			 "Invalid command", msg->data, msg->data_size);
 
 		rcu_read_lock();
 		if (!intf->in_shutdown) {
@@ -4217,7 +4200,7 @@ static int handle_one_recv_msg(struct ipmi_smi *intf,
 	int requeue;
 	int chan;
 
-	ipmi_debug_msg("Recv:", msg->rsp, msg->rsp_size);
+	pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
 
 	if ((msg->data_size >= 2)
 	    && (msg->data[0] == (IPMI_NETFN_APP_REQUEST << 2))
@@ -4576,7 +4559,7 @@ smi_from_recv_msg(struct ipmi_smi *intf, struct ipmi_recv_msg *recv_msg,
 	smi_msg->data_size = recv_msg->msg.data_len;
 	smi_msg->msgid = STORE_SEQ_IN_MSGID(seq, seqid);
 
-	ipmi_debug_msg("Resend: ", smi_msg->data, smi_msg->data_size);
+	pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
 
 	return smi_msg;
 }



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph
  2019-10-11 15:46       ` [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph Joe Perches
@ 2019-10-11 16:05         ` Andy Shevchenko
  2019-10-11 16:15           ` Joe Perches
  2019-10-14  0:55         ` kbuild test robot
  2019-10-14  1:21         ` kbuild test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2019-10-11 16:05 UTC (permalink / raw)
  To: Joe Perches; +Cc: Corey Minyard, openipmi-developer, linux-kernel

On Fri, Oct 11, 2019 at 08:46:41AM -0700, Joe Perches wrote:
> Using %*ph format to print small buffers as hex string reduces
> overall object size and allows the removal of the two variants
> of ipmi_debug_msg.
> 
> This also removes unnecessary duplicate colons from output when
> enabled by #DEBUG or newly possible CONFIG_DYNAMIC_DEBUG.

I have sent v2 with slightly better approach (no need to have %s).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph
  2019-10-11 16:05         ` Andy Shevchenko
@ 2019-10-11 16:15           ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2019-10-11 16:15 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Corey Minyard, openipmi-developer, linux-kernel

On Fri, 2019-10-11 at 19:05 +0300, Andy Shevchenko wrote:
> On Fri, Oct 11, 2019 at 08:46:41AM -0700, Joe Perches wrote:
> > Using %*ph format to print small buffers as hex string reduces
> > overall object size and allows the removal of the two variants
> > of ipmi_debug_msg.
> > 
> > This also removes unnecessary duplicate colons from output when
> > enabled by #DEBUG or newly possible CONFIG_DYNAMIC_DEBUG.
> 
> I have sent v2 with slightly better approach (no need to have %s).

Great, yours has the advantage of actually compiling properly.
I did not reverse the arguments after the format.  oops.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph
  2019-10-11 15:46       ` [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph Joe Perches
  2019-10-11 16:05         ` Andy Shevchenko
@ 2019-10-14  0:55         ` kbuild test robot
  2019-10-14  1:21         ` kbuild test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2019-10-14  0:55 UTC (permalink / raw)
  To: Joe Perches
  Cc: kbuild-all, Andy Shevchenko, Corey Minyard, openipmi-developer,
	linux-kernel

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

Hi Joe,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[cannot apply to v5.4-rc2 next-20191011]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Joe-Perches/ipmi-Convert-ipmi_debug_msg-to-pr_debug-and-use-ph/20191014-051352
config: x86_64-kexec (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
   drivers/char/ipmi/ipmi_msghandler.c: In function 'i_ipmi_request':
>> include/linux/kern_levels.h:5:18: warning: field width specifier '*' expects argument of type 'int', but argument 4 has type 'unsigned char *' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:2251:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:2251:18: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~^~
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
>> include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 5 has type 'int' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:2251:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:2251:19: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~~^
                    %*d
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
   drivers/char/ipmi/ipmi_msghandler.c: In function 'handle_ipmb_get_msg_cmd':
>> include/linux/kern_levels.h:5:18: warning: field width specifier '*' expects argument of type 'int', but argument 4 has type 'unsigned char *' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:18: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~^~
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
>> include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 5 has type 'int' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:19: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~~^
                    %*d
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
   drivers/char/ipmi/ipmi_msghandler.c: In function 'handle_one_recv_msg':
>> include/linux/kern_levels.h:5:18: warning: field width specifier '*' expects argument of type 'int', but argument 4 has type 'unsigned char *' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:17: note: format string is defined here
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
                   ~^~
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
>> include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 5 has type 'int' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:18: note: format string is defined here
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
                   ~~^
                   %*d
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
   drivers/char/ipmi/ipmi_msghandler.c: In function 'smi_from_recv_msg':
>> include/linux/kern_levels.h:5:18: warning: field width specifier '*' expects argument of type 'int', but argument 4 has type 'unsigned char *' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:17: note: format string is defined here
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
                   ~^~
   In file included from include/linux/kernel.h:15:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from drivers/char/ipmi/ipmi_msghandler.c:17:
>> include/linux/kern_levels.h:5:18: warning: format '%p' expects argument of type 'void *', but argument 5 has type 'int' [-Wformat=]
    #define KERN_SOH "\001"  /* ASCII Start Of Header */
                     ^
   include/linux/printk.h:137:10: note: in definition of macro 'no_printk'
      printk(fmt, ##__VA_ARGS__);  \
             ^~~
   include/linux/kern_levels.h:15:20: note: in expansion of macro 'KERN_SOH'
    #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
                       ^~~~~~~~
   include/linux/printk.h:342:12: note: in expansion of macro 'KERN_DEBUG'
     no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
               ^~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:18: note: format string is defined here
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
                   ~~^
                   %*d

vim +5 include/linux/kern_levels.h

314ba3520e513a Joe Perches 2012-07-30  4  
04d2c8c83d0e3a Joe Perches 2012-07-30 @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30  6  #define KERN_SOH_ASCII	'\001'
04d2c8c83d0e3a Joe Perches 2012-07-30  7  

:::::: The code at line 5 was first introduced by commit
:::::: 04d2c8c83d0e3ac5f78aeede51babb3236200112 printk: convert the format for KERN_<LEVEL> to a 2 byte pattern

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27480 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph
  2019-10-11 15:46       ` [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph Joe Perches
  2019-10-11 16:05         ` Andy Shevchenko
  2019-10-14  0:55         ` kbuild test robot
@ 2019-10-14  1:21         ` kbuild test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2019-10-14  1:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: kbuild-all, Andy Shevchenko, Corey Minyard, openipmi-developer,
	linux-kernel

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

Hi Joe,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[cannot apply to v5.4-rc2 next-20191011]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Joe-Perches/ipmi-Convert-ipmi_debug_msg-to-pr_debug-and-use-ph/20191014-051352
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sparc64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/char/ipmi/ipmi_msghandler.c: In function 'i_ipmi_request':
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: field width specifier '*' expects argument of type 'int', but argument 5 has type 'unsigned char *' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
>> drivers/char/ipmi/ipmi_msghandler.c:2251:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:2251:18: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~^~
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: format '%p' expects argument of type 'void *', but argument 6 has type 'int' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
>> drivers/char/ipmi/ipmi_msghandler.c:2251:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:2251:19: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~~^
                    %*d
   drivers/char/ipmi/ipmi_msghandler.c: In function 'handle_ipmb_get_msg_cmd':
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: field width specifier '*' expects argument of type 'int', but argument 5 has type 'unsigned char *' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:18: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~^~
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: format '%p' expects argument of type 'void *', but argument 6 has type 'int' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:3: note: in expansion of macro 'pr_debug'
      pr_debug("%s: %*ph\n",
      ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:3715:19: note: format string is defined here
      pr_debug("%s: %*ph\n",
                    ~~^
                    %*d
   drivers/char/ipmi/ipmi_msghandler.c: In function 'handle_one_recv_msg':
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: field width specifier '*' expects argument of type 'int', but argument 5 has type 'unsigned char *' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:17: note: format string is defined here
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
                   ~^~
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: format '%p' expects argument of type 'void *', but argument 6 has type 'int' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4203:18: note: format string is defined here
     pr_debug("%s: %*ph\n", "Recv", msg->rsp, msg->rsp_size);
                   ~~^
                   %*d
   drivers/char/ipmi/ipmi_msghandler.c: In function 'smi_from_recv_msg':
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: field width specifier '*' expects argument of type 'int', but argument 5 has type 'unsigned char *' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:17: note: format string is defined here
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
                   ~^~
>> drivers/char/ipmi/ipmi_msghandler.c:14:21: warning: format '%p' expects argument of type 'void *', but argument 6 has type 'int' [-Wformat=]
    #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
                        ^
   include/linux/dynamic_debug.h:125:15: note: in expansion of macro 'pr_fmt'
      func(&id, ##__VA_ARGS__);  \
                  ^~~~~~~~~~~
   include/linux/dynamic_debug.h:143:2: note: in expansion of macro '__dynamic_func_call'
     __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:153:2: note: in expansion of macro '_dynamic_func_call'
     _dynamic_func_call(fmt, __dynamic_pr_debug,  \
     ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
     dynamic_pr_debug(fmt, ##__VA_ARGS__)
     ^~~~~~~~~~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:2: note: in expansion of macro 'pr_debug'
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
     ^~~~~~~~
   drivers/char/ipmi/ipmi_msghandler.c:4562:18: note: format string is defined here
     pr_debug("%s: %*ph\n", "Resend", smi_msg->data, smi_msg->data_size);
                   ~~^
                   %*d

vim +14 drivers/char/ipmi/ipmi_msghandler.c

445e2cbda928a3 Joe Perches 2018-05-09 @14  #define pr_fmt(fmt) "%s" fmt, "IPMI message handler: "
445e2cbda928a3 Joe Perches 2018-05-09  15  #define dev_fmt pr_fmt
445e2cbda928a3 Joe Perches 2018-05-09  16  

:::::: The code at line 14 was first introduced by commit
:::::: 445e2cbda928a3523c1c1da76788d19df52611c8 ipmi: msghandler: Add and use pr_fmt and dev_fmt, remove PFX

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Corey Minyard <cminyard@mvista.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59075 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-10-14  1:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-11 14:52 [PATCH v1] ipmi: use %*ph to print small buffer Andy Shevchenko
2019-10-11 14:58 ` Joe Perches
2019-10-11 15:12   ` Andy Shevchenko
2019-10-11 15:18     ` Joe Perches
2019-10-11 15:36       ` Andy Shevchenko
2019-10-11 15:46       ` [PATCH] ipmi: Convert ipmi_debug_msg to pr_debug and use %*ph Joe Perches
2019-10-11 16:05         ` Andy Shevchenko
2019-10-11 16:15           ` Joe Perches
2019-10-14  0:55         ` kbuild test robot
2019-10-14  1:21         ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox