2010-07-06 04:25:51

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: build warning after merge of the net tree

Hi Dave,

After merging the net tree, today's linux-next build (powerpc
ppc64_defconfig) produced these warnings:

drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
drivers/scsi/sym53c8xx_2/sym_hipd.c:78: warning: zero-length gnu_printf format string
drivers/scsi/constants.c: In function 'scsi_print_sense':
drivers/scsi/constants.c:1407: warning: zero-length gnu_printf format string
drivers/scsi/constants.c:1413: warning: zero-length gnu_printf format string
drivers/scsi/constants.c: In function 'scsi_print_result':
drivers/scsi/constants.c:1456: warning: zero-length gnu_printf format string
drivers/scsi/sd.c: In function 'sd_print_sense_hdr':
drivers/scsi/sd.c:2599: warning: zero-length gnu_printf format string
drivers/scsi/sd.c:2601: warning: zero-length gnu_printf format string
drivers/scsi/sd.c: In function 'sd_print_result':
drivers/scsi/sd.c:2607: warning: zero-length gnu_printf format string

(There may be more ...)

Introduced by commit 99bcf217183e02ebae46373896fba7f12d588001 ("device.h
drivers/base/core.c Convert dev_<level> logging macros to functions").
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (1.17 kB)
(No filename) (490.00 B)
Download all attachments

2010-07-08 00:45:12

by David Miller

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

From: Stephen Rothwell <[email protected]>
Date: Tue, 6 Jul 2010 14:25:42 +1000

> After merging the net tree, today's linux-next build (powerpc
> ppc64_defconfig) produced these warnings:
>
> drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
> drivers/scsi/sym53c8xx_2/sym_hipd.c:78: warning: zero-length gnu_printf format string

Thanks Stephen I'll look into this.

2010-07-08 01:18:35

by David Miller

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

From: David Miller <[email protected]>
Date: Wed, 07 Jul 2010 17:45:22 -0700 (PDT)

> From: Stephen Rothwell <[email protected]>
> Date: Tue, 6 Jul 2010 14:25:42 +1000
>
>> After merging the net tree, today's linux-next build (powerpc
>> ppc64_defconfig) produced these warnings:
>>
>> drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
>> drivers/scsi/sym53c8xx_2/sym_hipd.c:78: warning: zero-length gnu_printf format string
>
> Thanks Stephen I'll look into this.

Yeah this is a bit ugly.

It used to be that the dev_*() format string was CPP pasted to whatever
format string the user gave. So if the user gave an empty string it
still looked like a non-empty printf string.

But that no longer happens because we hide the implementation, and thus
the top-level printf format string, in the external functions.

It seems the construction:

/*
* Stupid hackaround for existing uses of non-printk uses dev_info
*
* Note that the definition of dev_info below is actually _dev_info
* and a macro is used to avoid redefining dev_info
*/

#define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)

added to linux/device.h was meant to handle these cases, but as we see
it doesn't.

It looks like there are just a hand-ful of cases, so maybe we can tweak
them by hand. For example, in the sym53c8xx_2 driver bits we can replace
the NULL labels passed to sym_print_msg() with a real string and therefore
remove the "" case.

Joe, any better ideas?

2010-07-08 04:13:49

by Joe Perches

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

On Wed, 2010-07-07 at 18:18 -0700, David Miller wrote:
> From: David Miller <[email protected]>
> Date: Wed, 07 Jul 2010 17:45:22 -0700 (PDT)
> > From: Stephen Rothwell <[email protected]>
> > Date: Tue, 6 Jul 2010 14:25:42 +1000
> >> After merging the net tree, today's linux-next build (powerpc
> >> ppc64_defconfig) produced these warnings:
> >> drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
> >> drivers/scsi/sym53c8xx_2/sym_hipd.c:78: warning: zero-length gnu_printf format string
> > Thanks Stephen I'll look into this.
> Yeah this is a bit ugly.
>
> It used to be that the dev_*() format string was CPP pasted to whatever
> format string the user gave. So if the user gave an empty string it
> still looked like a non-empty printf string.
>
> But that no longer happens because we hide the implementation, and thus
> the top-level printf format string, in the external functions.
>
> It seems the construction:
>
> /*
> * Stupid hackaround for existing uses of non-printk uses dev_info
> *
> * Note that the definition of dev_info below is actually _dev_info
> * and a macro is used to avoid redefining dev_info
> */
>
> #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
>
> added to linux/device.h was meant to handle these cases, but as we see
> it doesn't.

Nope, the _dev_info/dev_info is meant to handle the
current uses of dev_info as a variable like this one:

$ grep dev_info drivers/net/pcmcia/pcnet_cs.c
static dev_info_t dev_info = "pcnet_cs";
ret = request_irq(dev->irq, ei_irq_wrapper, IRQF_SHARED, dev_info, dev);

Without the _dev_info and dev_info as a macro,
the function is redefined as a variable.

> It looks like there are just a hand-ful of cases, so maybe we can tweak
> them by hand. For example, in the sym53c8xx_2 driver bits we can replace
> the NULL labels passed to sym_print_msg() with a real string and therefore
> remove the "" case.
>
> Joe, any better ideas?

You're right there are just a few cases where dev_info
is uses as a preface for a hex_dump style display.

Maybe it'd be OK to simply add a trailing space to the
preface and remove any leading spaces from the subsequent
initial printks.

dev_info(dev, " ");

2010-07-11 02:51:57

by David Miller

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

From: Joe Perches <[email protected]>
Date: Wed, 07 Jul 2010 21:13:42 -0700

> On Wed, 2010-07-07 at 18:18 -0700, David Miller wrote:
>> It looks like there are just a hand-ful of cases, so maybe we can tweak
>> them by hand. For example, in the sym53c8xx_2 driver bits we can replace
>> the NULL labels passed to sym_print_msg() with a real string and therefore
>> remove the "" case.
>>
>> Joe, any better ideas?
>
> You're right there are just a few cases where dev_info
> is uses as a preface for a hex_dump style display.
>
> Maybe it'd be OK to simply add a trailing space to the
> preface and remove any leading spaces from the subsequent
> initial printks.
>
> dev_info(dev, " ");

That might work.

The sym53c8xx_2 doesn't even need this, like I said, you could
just remove the NULL 'label' argument cases and then have that
bit cured.

Could you take a stab at this and the other scsi bits that
trigger this warning?

Thanks Joe!

2010-07-11 05:08:46

by Joe Perches

[permalink] [raw]
Subject: [PATCH net-next] drivers/scsi: Remove warnings after vsprintf %pV introduction

On Sat, 2010-07-10 at 19:52 -0700, David Miller wrote:
> Could you take a stab at this and the other scsi bits that
> trigger this warning?

Remove warnings introduced by conversions of dev_<level>
macros to functions.

Compile tested only.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/scsi/constants.c | 63 ++++++++++++++++++++---------------
drivers/scsi/sd.c | 6 ++--
drivers/scsi/sym53c8xx_2/sym_hipd.c | 10 ++---
3 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index cd05e04..f95de51 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1226,29 +1226,38 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq) {
}
EXPORT_SYMBOL(scsi_extd_sense_format);

+static void scsi_show_extd_sense_args(const char *fmt, ...)
+{
+ va_list args;
+ struct va_format vaf;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_CONT "Add. Sense: %pV\n", &vaf);
+
+ va_end(args);
+}
+
void
scsi_show_extd_sense(unsigned char asc, unsigned char ascq)
{
const char *extd_sense_fmt = scsi_extd_sense_format(asc, ascq);

if (extd_sense_fmt) {
- if (strstr(extd_sense_fmt, "%x")) {
- printk("Add. Sense: ");
- printk(extd_sense_fmt, ascq);
- } else
- printk("Add. Sense: %s", extd_sense_fmt);
+ scsi_show_extd_sense_args(extd_sense_fmt, ascq);
} else {
if (asc >= 0x80)
- printk("<<vendor>> ASC=0x%x ASCQ=0x%x", asc,
- ascq);
+ printk(KERN_CONT "<<vendor>> ASC=0x%x ASCQ=0x%x",
+ asc, ascq);
if (ascq >= 0x80)
- printk("ASC=0x%x <<vendor>> ASCQ=0x%x", asc,
- ascq);
+ printk(KERN_CONT "ASC=0x%x <<vendor>> ASCQ=0x%x\n",
+ asc, ascq);
else
- printk("ASC=0x%x ASCQ=0x%x", asc, ascq);
+ printk(KERN_CONT "ASC=0x%x ASCQ=0x%x\n", asc, ascq);
}
-
- printk("\n");
}
EXPORT_SYMBOL(scsi_show_extd_sense);

@@ -1310,15 +1319,15 @@ scsi_decode_sense_buffer(const unsigned char *sense_buffer, int sense_len,
if (0 == res) {
/* this may be SCSI-1 sense data */
num = (sense_len < 32) ? sense_len : 32;
- printk("Unrecognized sense data (in hex):");
+ printk(KERN_CONT "Unrecognized sense data (in hex):");
for (k = 0; k < num; ++k) {
if (0 == (k % 16)) {
- printk("\n");
- printk(KERN_INFO " ");
+ printk(KERN_CONT "\n");
+ printk(KERN_INFO " ");
}
- printk("%02x ", sense_buffer[k]);
+ printk(KERN_CONT " %02x", sense_buffer[k]);
}
- printk("\n");
+ printk(KERN_CONT "\n");
return;
}
}
@@ -1364,22 +1373,22 @@ scsi_decode_sense_extras(const unsigned char *sense_buffer, int sense_len,
res += snprintf(buff + res, blen - res, "ILI");
}
if (res > 0)
- printk("%s\n", buff);
+ printk(KERN_CONT "%s\n", buff);
} else if (sshdr->additional_length > 0) {
/* descriptor format with sense descriptors */
num = 8 + sshdr->additional_length;
num = (sense_len < num) ? sense_len : num;
- printk("Descriptor sense data with sense descriptors "
+ printk(KERN_CONT "Descriptor sense data with sense descriptors "
"(in hex):");
for (k = 0; k < num; ++k) {
if (0 == (k % 16)) {
- printk("\n");
- printk(KERN_INFO " ");
+ printk(KERN_CONT "\n");
+ printk(KERN_INFO " ");
}
- printk("%02x ", sense_buffer[k]);
+ printk(KERN_CONT " %02x", sense_buffer[k]);
}

- printk("\n");
+ printk(KERN_CONT "\n");
}

}
@@ -1404,13 +1413,13 @@ void scsi_print_sense(char *name, struct scsi_cmnd *cmd)
{
struct scsi_sense_hdr sshdr;

- scmd_printk(KERN_INFO, cmd, "");
+ scmd_printk(KERN_INFO, cmd, " ");
scsi_decode_sense_buffer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
&sshdr);
scsi_show_sense_hdr(&sshdr);
scsi_decode_sense_extras(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
&sshdr);
- scmd_printk(KERN_INFO, cmd, "");
+ scmd_printk(KERN_INFO, cmd, " ");
scsi_show_extd_sense(sshdr.asc, sshdr.ascq);
}
EXPORT_SYMBOL(scsi_print_sense);
@@ -1443,7 +1452,7 @@ void scsi_show_result(int result)

void scsi_show_result(int result)
{
- printk("Result: hostbyte=0x%02x driverbyte=0x%02x\n",
+ printk(KERN_CONT "Result: hostbyte=0x%02x driverbyte=0x%02x\n",
host_byte(result), driver_byte(result));
}

@@ -1453,7 +1462,7 @@ EXPORT_SYMBOL(scsi_show_result);

void scsi_print_result(struct scsi_cmnd *cmd)
{
- scmd_printk(KERN_INFO, cmd, "");
+ scmd_printk(KERN_INFO, cmd, " ");
scsi_show_result(cmd->result);
}
EXPORT_SYMBOL(scsi_print_result);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 829cc37..2fddadd 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2550,15 +2550,15 @@ module_exit(exit_sd);
static void sd_print_sense_hdr(struct scsi_disk *sdkp,
struct scsi_sense_hdr *sshdr)
{
- sd_printk(KERN_INFO, sdkp, "");
+ sd_printk(KERN_INFO, sdkp, " ");
scsi_show_sense_hdr(sshdr);
- sd_printk(KERN_INFO, sdkp, "");
+ sd_printk(KERN_INFO, sdkp, " ");
scsi_show_extd_sense(sshdr->asc, sshdr->ascq);
}

static void sd_print_result(struct scsi_disk *sdkp, int result)
{
- sd_printk(KERN_INFO, sdkp, "");
+ sd_printk(KERN_INFO, sdkp, " ");
scsi_show_result(result);
}

diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index a7bc8b7..d740a5b 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -72,10 +72,7 @@ static void sym_printl_hex(u_char *p, int n)

static void sym_print_msg(struct sym_ccb *cp, char *label, u_char *msg)
{
- if (label)
- sym_print_addr(cp->cmd, "%s: ", label);
- else
- sym_print_addr(cp->cmd, "");
+ sym_print_addr(cp->cmd, "%s: ", label);

spi_print_msg(msg);
printf("\n");
@@ -4558,7 +4555,8 @@ static void sym_int_sir(struct sym_hcb *np)
switch (np->msgin [2]) {
case M_X_MODIFY_DP:
if (DEBUG_FLAGS & DEBUG_POINTER)
- sym_print_msg(cp, NULL, np->msgin);
+ sym_print_msg(cp, "extended msg ",
+ np->msgin);
tmp = (np->msgin[3]<<24) + (np->msgin[4]<<16) +
(np->msgin[5]<<8) + (np->msgin[6]);
sym_modify_dp(np, tp, cp, tmp);
@@ -4585,7 +4583,7 @@ static void sym_int_sir(struct sym_hcb *np)
*/
case M_IGN_RESIDUE:
if (DEBUG_FLAGS & DEBUG_POINTER)
- sym_print_msg(cp, NULL, np->msgin);
+ sym_print_msg(cp, "half byte ", np->msgin);
if (cp->host_flags & HF_SENSE)
OUTL_DSP(np, SCRIPTA_BA(np, clrack));
else


2010-07-11 06:10:15

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next] drivers/scsi: Remove warnings after vsprintf %pV introduction

From: Joe Perches <[email protected]>
Date: Sat, 10 Jul 2010 22:08:41 -0700

> On Sat, 2010-07-10 at 19:52 -0700, David Miller wrote:
>> Could you take a stab at this and the other scsi bits that
>> trigger this warning?
>
> Remove warnings introduced by conversions of dev_<level>
> macros to functions.
>
> Compile tested only.
>
> Signed-off-by: Joe Perches <[email protected]>

SCSI folks, the background is that we have moved the dev_*() printk
macros to external functions, so that the prefixing printf strings
don't get emitting at every call site.

As a consequence, dev_*() calls that try to use an empty string as the
printf format emit a warning from gcc since an empty constant string
is not a valid printf format.

That's what this change is all about.

Anyways:

Acked-by: David S. Miller <[email protected]>

2010-07-12 08:27:24

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH net-next] drivers/scsi: Remove warnings after vsprintf %pV introduction

On Sat, 2010-07-10 at 23:10 -0700, David Miller wrote:
> From: Joe Perches <[email protected]>
> Date: Sat, 10 Jul 2010 22:08:41 -0700
>
> > On Sat, 2010-07-10 at 19:52 -0700, David Miller wrote:
> >> Could you take a stab at this and the other scsi bits that
> >> trigger this warning?
> >
> > Remove warnings introduced by conversions of dev_<level>
> > macros to functions.
> >
> > Compile tested only.
> >
> > Signed-off-by: Joe Perches <[email protected]>
>
> SCSI folks, the background is that we have moved the dev_*() printk
> macros to external functions, so that the prefixing printf strings
> don't get emitting at every call site.
>
> As a consequence, dev_*() calls that try to use an empty string as the
> printf format emit a warning from gcc since an empty constant string
> is not a valid printf format.
>
> That's what this change is all about.

Thanks, that explains the "" -> " " conversions.

What's the other 60% of the patch about? the strange addition of
scsi_show_extd_sense_args() and all the KERN_CONT bits?

James

2010-07-13 03:36:58

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next] drivers/scsi: Remove warnings after vsprintf %pV introduction

From: James Bottomley <[email protected]>
Date: Mon, 12 Jul 2010 04:27:18 -0400

> What's the other 60% of the patch about? the strange addition of
> scsi_show_extd_sense_args() and all the KERN_CONT bits?

Well the new function is for logical seperation, and KERN_CONT
is what is supposed to be at the front of every printk format
that continues a partially-printed line.

2010-07-27 13:12:33

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH net-next] drivers/scsi: Remove warnings after vsprintf %pV introduction

On Sat, 2010-07-10 at 22:08 -0700, Joe Perches wrote:
> On Sat, 2010-07-10 at 19:52 -0700, David Miller wrote:
> > Could you take a stab at this and the other scsi bits that
> > trigger this warning?
>
> Remove warnings introduced by conversions of dev_<level>
> macros to functions.
>
> Compile tested only.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> drivers/scsi/constants.c | 63 ++++++++++++++++++++---------------
> drivers/scsi/sd.c | 6 ++--
> drivers/scsi/sym53c8xx_2/sym_hipd.c | 10 ++---
> 3 files changed, 43 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index cd05e04..f95de51 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -1226,29 +1226,38 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq) {
> }
> EXPORT_SYMBOL(scsi_extd_sense_format);
>
> +static void scsi_show_extd_sense_args(const char *fmt, ...)
> +{
> + va_list args;
> + struct va_format vaf;
> +
> + va_start(args, fmt);
> +
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + printk(KERN_CONT "Add. Sense: %pV\n", &vaf);
> +
> + va_end(args);
> +}
> +

This doesn't have a place in the patch, it's an unnecessary conversion

> void
> scsi_show_extd_sense(unsigned char asc, unsigned char ascq)
> {
> const char *extd_sense_fmt = scsi_extd_sense_format(asc, ascq);
>
> if (extd_sense_fmt) {
> - if (strstr(extd_sense_fmt, "%x")) {
> - printk("Add. Sense: ");
> - printk(extd_sense_fmt, ascq);
> - } else
> - printk("Add. Sense: %s", extd_sense_fmt);
> + scsi_show_extd_sense_args(extd_sense_fmt, ascq);
> } else {
> if (asc >= 0x80)
> - printk("<<vendor>> ASC=0x%x ASCQ=0x%x", asc,
> - ascq);
> + printk(KERN_CONT "<<vendor>> ASC=0x%x ASCQ=0x%x",
> + asc, ascq);
> if (ascq >= 0x80)
> - printk("ASC=0x%x <<vendor>> ASCQ=0x%x", asc,
> - ascq);
> + printk(KERN_CONT "ASC=0x%x <<vendor>> ASCQ=0x%x\n",
> + asc, ascq);
> else
> - printk("ASC=0x%x ASCQ=0x%x", asc, ascq);
> + printk(KERN_CONT "ASC=0x%x ASCQ=0x%x\n", asc, ascq);

And half these KERN_CONT additions are spurious since you'd not
otherwise touch the line

> }
> -
> - printk("\n");
> }
> EXPORT_SYMBOL(scsi_show_extd_sense);
>
> @@ -1310,15 +1319,15 @@ scsi_decode_sense_buffer(const unsigned char *sense_buffer, int sense_len,
> if (0 == res) {
> /* this may be SCSI-1 sense data */
> num = (sense_len < 32) ? sense_len : 32;
> - printk("Unrecognized sense data (in hex):");
> + printk(KERN_CONT "Unrecognized sense data (in hex):");
> for (k = 0; k < num; ++k) {
> if (0 == (k % 16)) {
> - printk("\n");
> - printk(KERN_INFO " ");
> + printk(KERN_CONT "\n");
> + printk(KERN_INFO " ");
> }
> - printk("%02x ", sense_buffer[k]);
> + printk(KERN_CONT " %02x", sense_buffer[k]);
> }
> - printk("\n");
> + printk(KERN_CONT "\n");
> return;
> }
> }
> @@ -1364,22 +1373,22 @@ scsi_decode_sense_extras(const unsigned char *sense_buffer, int sense_len,
> res += snprintf(buff + res, blen - res, "ILI");
> }
> if (res > 0)
> - printk("%s\n", buff);
> + printk(KERN_CONT "%s\n", buff);
> } else if (sshdr->additional_length > 0) {
> /* descriptor format with sense descriptors */
> num = 8 + sshdr->additional_length;
> num = (sense_len < num) ? sense_len : num;
> - printk("Descriptor sense data with sense descriptors "
> + printk(KERN_CONT "Descriptor sense data with sense descriptors "
> "(in hex):");
> for (k = 0; k < num; ++k) {
> if (0 == (k % 16)) {
> - printk("\n");
> - printk(KERN_INFO " ");
> + printk(KERN_CONT "\n");
> + printk(KERN_INFO " ");
> }
> - printk("%02x ", sense_buffer[k]);
> + printk(KERN_CONT " %02x", sense_buffer[k]);
> }
>
> - printk("\n");
> + printk(KERN_CONT "\n");
> }
>
> }
> @@ -1404,13 +1413,13 @@ void scsi_print_sense(char *name, struct scsi_cmnd *cmd)
> {
> struct scsi_sense_hdr sshdr;
>
> - scmd_printk(KERN_INFO, cmd, "");
> + scmd_printk(KERN_INFO, cmd, " ");
> scsi_decode_sense_buffer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
> &sshdr);
> scsi_show_sense_hdr(&sshdr);
> scsi_decode_sense_extras(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
> &sshdr);
> - scmd_printk(KERN_INFO, cmd, "");
> + scmd_printk(KERN_INFO, cmd, " ");
> scsi_show_extd_sense(sshdr.asc, sshdr.ascq);
> }
> EXPORT_SYMBOL(scsi_print_sense);
> @@ -1443,7 +1452,7 @@ void scsi_show_result(int result)
>
> void scsi_show_result(int result)
> {
> - printk("Result: hostbyte=0x%02x driverbyte=0x%02x\n",
> + printk(KERN_CONT "Result: hostbyte=0x%02x driverbyte=0x%02x\n",
> host_byte(result), driver_byte(result));
> }
>
> @@ -1453,7 +1462,7 @@ EXPORT_SYMBOL(scsi_show_result);
>
> void scsi_print_result(struct scsi_cmnd *cmd)
> {
> - scmd_printk(KERN_INFO, cmd, "");
> + scmd_printk(KERN_INFO, cmd, " ");
> scsi_show_result(cmd->result);
> }
> EXPORT_SYMBOL(scsi_print_result);
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 829cc37..2fddadd 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2550,15 +2550,15 @@ module_exit(exit_sd);
> static void sd_print_sense_hdr(struct scsi_disk *sdkp,
> struct scsi_sense_hdr *sshdr)
> {
> - sd_printk(KERN_INFO, sdkp, "");
> + sd_printk(KERN_INFO, sdkp, " ");
> scsi_show_sense_hdr(sshdr);
> - sd_printk(KERN_INFO, sdkp, "");
> + sd_printk(KERN_INFO, sdkp, " ");
> scsi_show_extd_sense(sshdr->asc, sshdr->ascq);
> }
>
> static void sd_print_result(struct scsi_disk *sdkp, int result)
> {
> - sd_printk(KERN_INFO, sdkp, "");
> + sd_printk(KERN_INFO, sdkp, " ");
> scsi_show_result(result);
> }
>
> diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
> index a7bc8b7..d740a5b 100644
> --- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
> +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
> @@ -72,10 +72,7 @@ static void sym_printl_hex(u_char *p, int n)
>
> static void sym_print_msg(struct sym_ccb *cp, char *label, u_char *msg)
> {
> - if (label)
> - sym_print_addr(cp->cmd, "%s: ", label);
> - else
> - sym_print_addr(cp->cmd, "");
> + sym_print_addr(cp->cmd, "%s: ", label);
>
> spi_print_msg(msg);
> printf("\n");
> @@ -4558,7 +4555,8 @@ static void sym_int_sir(struct sym_hcb *np)
> switch (np->msgin [2]) {
> case M_X_MODIFY_DP:
> if (DEBUG_FLAGS & DEBUG_POINTER)
> - sym_print_msg(cp, NULL, np->msgin);
> + sym_print_msg(cp, "extended msg ",
> + np->msgin);
> tmp = (np->msgin[3]<<24) + (np->msgin[4]<<16) +
> (np->msgin[5]<<8) + (np->msgin[6]);
> sym_modify_dp(np, tp, cp, tmp);
> @@ -4585,7 +4583,7 @@ static void sym_int_sir(struct sym_hcb *np)
> */
> case M_IGN_RESIDUE:
> if (DEBUG_FLAGS & DEBUG_POINTER)
> - sym_print_msg(cp, NULL, np->msgin);
> + sym_print_msg(cp, "half byte ", np->msgin);
> if (cp->host_flags & HF_SENSE)
> OUTL_DSP(np, SCRIPTA_BA(np, clrack));
> else

OK, so please resend on the necessary bits. That's "" -> " " in the
X_printk() statements. If you remove the space from a continuation
line, then KERN_CONT is appropriate to keep checkpatch quiet.

James

2010-08-31 02:57:48

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

Hi all,

On Tue, 6 Jul 2010 14:25:42 +1000 Stephen Rothwell <[email protected]> wrote:
>
> After merging the net tree, today's linux-next build (powerpc
> ppc64_defconfig) produced these warnings:
>
> drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
> drivers/scsi/sym53c8xx_2/sym_hipd.c:78: warning: zero-length gnu_printf format string
> drivers/scsi/constants.c: In function 'scsi_print_sense':
> drivers/scsi/constants.c:1407: warning: zero-length gnu_printf format string
> drivers/scsi/constants.c:1413: warning: zero-length gnu_printf format string
> drivers/scsi/constants.c: In function 'scsi_print_result':
> drivers/scsi/constants.c:1456: warning: zero-length gnu_printf format string
> drivers/scsi/sd.c: In function 'sd_print_sense_hdr':
> drivers/scsi/sd.c:2599: warning: zero-length gnu_printf format string
> drivers/scsi/sd.c:2601: warning: zero-length gnu_printf format string
> drivers/scsi/sd.c: In function 'sd_print_result':
> drivers/scsi/sd.c:2607: warning: zero-length gnu_printf format string
>
> (There may be more ...)
>
> Introduced by commit 99bcf217183e02ebae46373896fba7f12d588001 ("device.h
> drivers/base/core.c Convert dev_<level> logging macros to functions").

Can we have a fix for these please? They make too much noise in the builds.
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (1.37 kB)
(No filename) (490.00 B)
Download all attachments

2010-08-31 03:14:37

by Joe Perches

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

On Tue, 2010-08-31 at 12:57 +1000, Stephen Rothwell wrote:
> On Tue, 6 Jul 2010 14:25:42 +1000 Stephen Rothwell <[email protected]> wrote:
> > After merging the net tree, today's linux-next build (powerpc
> > ppc64_defconfig) produced these warnings:
> > drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
[]
> > Introduced by commit 99bcf217183e02ebae46373896fba7f12d588001 ("device.h
> > drivers/base/core.c Convert dev_<level> logging macros to functions").
> Can we have a fix for these please? They make too much noise in the builds.

Submitted July 7

https://patchwork.kernel.org/patch/111271/


2010-08-31 03:58:38

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

Hi Joe,

On Mon, 30 Aug 2010 20:14:34 -0700 Joe Perches <[email protected]> wrote:
>
> On Tue, 2010-08-31 at 12:57 +1000, Stephen Rothwell wrote:
> > On Tue, 6 Jul 2010 14:25:42 +1000 Stephen Rothwell <[email protected]> wrote:
> > > After merging the net tree, today's linux-next build (powerpc
> > > ppc64_defconfig) produced these warnings:
> > > drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
> []
> > > Introduced by commit 99bcf217183e02ebae46373896fba7f12d588001 ("device.h
> > > drivers/base/core.c Convert dev_<level> logging macros to functions").
> > Can we have a fix for these please? They make too much noise in the builds.
>
> Submitted July 7
>
> https://patchwork.kernel.org/patch/111271/

Yes, but there were comments on that and nothing since. I think that
James would like a minimal patch that just fixes the newly introduced
warnings.

--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/


Attachments:
(No filename) (984.00 B)
(No filename) (490.00 B)
Download all attachments

2010-08-31 04:03:08

by Joe Perches

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

On Tue, 2010-08-31 at 13:58 +1000, Stephen Rothwell wrote:
> On Mon, 30 Aug 2010 20:14:34 -0700 Joe Perches <[email protected]> wrote:
> > On Tue, 2010-08-31 at 12:57 +1000, Stephen Rothwell wrote:
> > > On Tue, 6 Jul 2010 14:25:42 +1000 Stephen Rothwell <[email protected]> wrote:
> > > > After merging the net tree, today's linux-next build (powerpc
> > > > ppc64_defconfig) produced these warnings:
> > > > drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
> > []
> > > > Introduced by commit 99bcf217183e02ebae46373896fba7f12d588001 ("device.h
> > > > drivers/base/core.c Convert dev_<level> logging macros to functions").
> > > Can we have a fix for these please? They make too much noise in the builds.
> > Submitted July 7
> > https://patchwork.kernel.org/patch/111271/
> Yes, but there were comments on that and nothing since. I think that
> James would like a minimal patch that just fixes the newly introduced
> warnings.

Hi Stephen.

What I submitted I think reasonable.
He can extract what he wants if he prefers it done that way.

cheers, Joe

2010-08-31 04:41:55

by David Miller

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

From: Stephen Rothwell <[email protected]>
Date: Tue, 31 Aug 2010 12:57:38 +1000

> Can we have a fix for these please? They make too much noise in the builds.

See below:

--------------------
Subject: Re: [GIT] Networking
From: David Miller <[email protected]>
To: [email protected]
Cc: [email protected], [email protected],
[email protected]
Date: Wed, 04 Aug 2010 13:41:15 -0700 (PDT)
X-Mailer: Mew version 6.3 on Emacs 23.1 / Mule 6.0 (HANACHIRUSATO)

From: Linus Torvalds <[email protected]>
Date: Wed, 4 Aug 2010 12:06:47 -0700

> On Tue, Aug 3, 2010 at 8:38 PM, David Miller <[email protected]> wrote:
>>
>> Another release, another merge window, another set of networking
>> changes to merge :-)
>
> Ok, merged. But you should double-check my merge resolution fixes,

Will do, thanks a lot.

> I'm also a bit unhappy about how it introduces new warnings in very
> subtle ways.

Joe Perches presented patches to fix this (arguably always broken)
issue to James Bottomley and co. several weeks ago:

http://marc.info/?l=linux-next&m=127882494322533&w=2

And it's been, in typical SCSI subsystem maintainer fastion, in
"stall" mode ever since.

In fact, when we noticed this problem, Joe Perches said he would only
post the patches to the SCSI folks if I would "deal" with James
Bottomley. If it's to that point, well... I don't know what to say.

I know the warning is introduced by changes in my tree, but I
figured with weeks until the merge window James would be OK with
Joe's fix for the warning by then. Perhaps I was wrong.

If under my control, I would never _ever_ let something like that
linger for weeks upon weeks in my tree. 24 hour resolution, tops.

And the same goes for Joe Perches, which is why I trust him and merged
his changes which triggered the warning in the first place. I can
always count on him to do something immediately to try and fix
fallout, or in the worst case say "ok we can't resolve this just
revert my original change".

So I'm sorry, I'll never create a situation again where the SCSI
maintainer is in the critical path for getting a warning fixed. :-)

2010-08-31 04:45:39

by David Miller

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

From: Stephen Rothwell <[email protected]>
Date: Tue, 31 Aug 2010 13:58:31 +1000

> Yes, but there were comments on that and nothing since. I think that
> James would like a minimal patch that just fixes the newly introduced
> warnings.

I think James is being overly difficult, and since it effects his code
you'd think that he might be a bit proactive and maybe spend the 5
freakin' minutes it might take to write the patch that matches his
exact requirements if they are so important.

Or, at least, I'm sure he could find enough time to sort it out in the
past month and a half don't you think?

I may be weird and a moron, but that's what I would do if my subsystem
had a build warning that was bugging people.

2010-08-31 04:46:28

by David Miller

[permalink] [raw]
Subject: Re: linux-next: build warning after merge of the net tree

From: Joe Perches <[email protected]>
Date: Mon, 30 Aug 2010 21:03:03 -0700

> On Tue, 2010-08-31 at 13:58 +1000, Stephen Rothwell wrote:
>> On Mon, 30 Aug 2010 20:14:34 -0700 Joe Perches <[email protected]> wrote:
>> > On Tue, 2010-08-31 at 12:57 +1000, Stephen Rothwell wrote:
>> > > On Tue, 6 Jul 2010 14:25:42 +1000 Stephen Rothwell <[email protected]> wrote:
>> > > > After merging the net tree, today's linux-next build (powerpc
>> > > > ppc64_defconfig) produced these warnings:
>> > > > drivers/scsi/sym53c8xx_2/sym_hipd.c: In function 'sym_print_msg':
>> > []
>> > > > Introduced by commit 99bcf217183e02ebae46373896fba7f12d588001 ("device.h
>> > > > drivers/base/core.c Convert dev_<level> logging macros to functions").
>> > > Can we have a fix for these please? They make too much noise in the builds.
>> > Submitted July 7
>> > https://patchwork.kernel.org/patch/111271/
>> Yes, but there were comments on that and nothing since. I think that
>> James would like a minimal patch that just fixes the newly introduced
>> warnings.
>
> Hi Stephen.
>
> What I submitted I think reasonable.

Me too.

> He can extract what he wants if he prefers it done that way.

Exactly, he's the maintainter so if he wants something fixed a
specific way he can do it as he likes.