Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 00/10] treewide: Fix format strings that misuse continuations

1 view
Skip to first unread message

Joe Perches

unread,
Jan 31, 2010, 3:10:03 PM1/31/10
to
Format strings that are continued with \ are frequently misused.
Change them to use mostly single line formats, some longer than 80 chars.
Fix a few miscellaneous typos at the same time.

Joe Perches (10):
arch/powerpc: Fix continuation line formats
arch/blackfin: Fix continuation line formats
drivers/ide: Fix continuation line formats
drivers/serial/bfin_5xx.c: Fix continuation line formats
drivers/scsi/arcmsr: Fix continuation line formats
drivers/staging: Fix continuation line formats
drivers/net/amd8111e.c: Fix continuation line formats
fs/proc/array.c: Fix continuation line formats
mm/slab.c: Fix continuation line formats
sound/soc/blackfin: Fix continuation line formats

arch/blackfin/mach-common/smp.c | 4 +-
arch/powerpc/kernel/nvram_64.c | 6 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c | 8 ++--
arch/powerpc/platforms/pseries/smp.c | 4 +-
drivers/ide/au1xxx-ide.c | 4 +-
drivers/ide/pmac.c | 4 +-
drivers/net/amd8111e.c | 3 +-
drivers/scsi/arcmsr/arcmsr_hba.c | 49 +++++++++----------
drivers/serial/bfin_5xx.c | 6 +--
drivers/staging/dream/qdsp5/audio_mp3.c | 3 +-
.../rtl8187se/ieee80211/ieee80211_softmac_wx.c | 3 +-
drivers/staging/rtl8187se/r8180_core.c | 3 +-
drivers/staging/sep/sep_driver.c | 3 +-
fs/proc/array.c | 7 ++-
mm/slab.c | 4 +-
sound/soc/blackfin/bf5xx-ac97-pcm.c | 8 +--
sound/soc/blackfin/bf5xx-i2s-pcm.c | 3 +-
sound/soc/blackfin/bf5xx-tdm-pcm.c | 3 +-
18 files changed, 55 insertions(+), 70 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Joe Perches

unread,
Jan 31, 2010, 3:10:02 PM1/31/10
to
String constants that are continued on subsequent lines with \
are not good.

The characters between seq_printf elements are tabs.
That was probably not intentional, but isn't being changed.
It's behind an #ifdef, so it could probably become a single space.

Signed-off-by: Joe Perches <j...@perches.com>
---
mm/slab.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 7451bda..9964619 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4228,8 +4228,8 @@ static int s_show(struct seq_file *m, void *p)
unsigned long node_frees = cachep->node_frees;
unsigned long overflows = cachep->node_overflow;

- seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu \
- %4lu %4lu %4lu %4lu %4lu", allocs, high, grown,
+ seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu %4lu %4lu %4lu %4lu %4lu",
+ allocs, high, grown,
reaped, errors, max_freeable, node_allocs,
node_frees, overflows);
}
--
1.6.6.rc0.57.gad7a

Joe Perches

unread,
Jan 31, 2010, 3:10:02 PM1/31/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---
drivers/net/amd8111e.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/amd8111e.c b/drivers/net/amd8111e.c
index 766aabf..c315480 100644
--- a/drivers/net/amd8111e.c
+++ b/drivers/net/amd8111e.c
@@ -1176,8 +1176,7 @@ static irqreturn_t amd8111e_interrupt(int irq, void *dev_id)
/* Schedule a polling routine */
__napi_schedule(&lp->napi);
} else if (intren0 & RINTEN0) {
- printk("************Driver bug! \
- interrupt while in poll\n");
+ printk("************Driver bug! interrupt while in poll\n");
/* Fix by disable receive interrupts */
writel(RINTEN0, mmio + INTEN0);
}
--
1.6.6.rc0.57.gad7a

Joe Perches

unread,
Jan 31, 2010, 3:10:03 PM1/31/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

arch/blackfin/mach-common/smp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/blackfin/mach-common/smp.c b/arch/blackfin/mach-common/smp.c
index 369e687..c7bcbe5 100644
--- a/arch/blackfin/mach-common/smp.c
+++ b/arch/blackfin/mach-common/smp.c
@@ -161,8 +161,8 @@ static irqreturn_t ipi_handler(int irq, void *dev_instance)
kfree(msg);
break;
default:
- printk(KERN_CRIT "CPU%u: Unknown IPI message \
- 0x%lx\n", cpu, msg->type);
+ printk(KERN_CRIT "CPU%u: Unknown IPI message 0x%lx\n",
+ cpu, msg->type);
kfree(msg);
break;
}
--
1.6.6.rc0.57.gad7a

Joe Perches

unread,
Jan 31, 2010, 3:10:03 PM1/31/10
to
String constants that are continued on subsequent lines with \
are not good.
Fix rebulid/rebuild typos

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/scsi/arcmsr/arcmsr_hba.c | 49 +++++++++++++++++--------------------
1 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 47d5d19..a0378d5 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -585,8 +585,8 @@ static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
break;
else {
retry_count--;
- printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' \
- timeout, retry count down = %d \n", acb->host->host_no, retry_count);
+ printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' timeout, retry count down = %d \n",
+ acb->host->host_no, retry_count);
}
} while (retry_count != 0);
}
@@ -602,8 +602,8 @@ static void arcmsr_flush_hbb_cache(struct AdapterControlBlock *acb)
break;
else {
retry_count--;
- printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' \
- timeout,retry count down = %d \n", acb->host->host_no, retry_count);
+ printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' timeout,retry count down = %d \n",
+ acb->host->host_no, retry_count);
}
} while (retry_count != 0);
}
@@ -732,12 +732,11 @@ static void arcmsr_drain_donequeue(struct AdapterControlBlock *acb, uint32_t fla
if (abortcmd) {
abortcmd->result |= DID_ABORT << 16;
arcmsr_ccb_complete(ccb, 1);
- printk(KERN_NOTICE "arcmsr%d: ccb ='0x%p' \
- isr got aborted command \n", acb->host->host_no, ccb);
+ printk(KERN_NOTICE "arcmsr%d: ccb ='0x%p' isr got aborted command \n",
+ acb->host->host_no, ccb);
}
}
- printk(KERN_NOTICE "arcmsr%d: isr get an illegal ccb command \
- done acb = '0x%p'"
+ printk(KERN_NOTICE "arcmsr%d: isr get an illegal ccb command done acb = '0x%p'"
"ccb = '0x%p' ccbacb = '0x%p' startdone = 0x%x"
" ccboutstandingcount = %d \n"
, acb->host->host_no
@@ -1001,7 +1000,7 @@ static void arcmsr_stop_hba_bgrb(struct AdapterControlBlock *acb)

if (arcmsr_hba_wait_msgint_ready(acb)) {
printk(KERN_NOTICE
- "arcmsr%d: wait 'stop adapter background rebulid' timeout \n"
+ "arcmsr%d: wait 'stop adapter background rebuild' timeout \n"
, acb->host->host_no);
}
}
@@ -1014,7 +1013,7 @@ static void arcmsr_stop_hbb_bgrb(struct AdapterControlBlock *acb)

if (arcmsr_hbb_wait_msgint_ready(acb)) {
printk(KERN_NOTICE
- "arcmsr%d: wait 'stop adapter background rebulid' timeout \n"
+ "arcmsr%d: wait 'stop adapter background rebuild' timeout \n"
, acb->host->host_no);
}
}
@@ -1709,8 +1708,8 @@ static void arcmsr_get_hba_config(struct AdapterControlBlock *acb)

writel(ARCMSR_INBOUND_MESG0_GET_CONFIG, &reg->inbound_msgaddr0);
if (arcmsr_hba_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
- miscellaneous data' timeout \n", acb->host->host_no);
+ printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware miscellaneous data' timeout \n",
+ acb->host->host_no);
}

count = 8;
@@ -1753,8 +1752,8 @@ static void arcmsr_get_hbb_config(struct AdapterControlBlock *acb)

writel(ARCMSR_MESSAGE_GET_CONFIG, reg->drv2iop_doorbell_reg);
if (arcmsr_hbb_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
- miscellaneous data' timeout \n", acb->host->host_no);
+ printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware miscellaneous data' timeout \n",
+ acb->host->host_no);
}

count = 8;
@@ -1889,8 +1888,7 @@ static void arcmsr_polling_hbb_ccbdone(struct AdapterControlBlock *acb,
poll_ccb_done = (ccb == poll_ccb) ? 1:0;
if ((ccb->acb != acb) || (ccb->startdone != ARCMSR_CCB_START)) {
if ((ccb->startdone == ARCMSR_CCB_ABORTED) || (ccb == poll_ccb)) {
- printk(KERN_NOTICE "arcmsr%d: \
- scsi id = %d lun = %d ccb = '0x%p' poll command abort successfully \n"
+ printk(KERN_NOTICE "arcmsr%d: scsi id = %d lun = %d ccb = '0x%p' poll command abort successfully \n"
,acb->host->host_no
,ccb->pcmd->device->id
,ccb->pcmd->device->lun
@@ -1958,8 +1956,7 @@ static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
writel(ARCMSR_INBOUND_MESG0_SET_CONFIG, \
&reg->inbound_msgaddr0);
if (arcmsr_hba_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: ""set ccb high \
- part physical address timeout\n",
+ printk(KERN_NOTICE "arcmsr%d: ""set ccb high part physical address timeout\n",
acb->host->host_no);
return 1;
}
@@ -1979,7 +1976,7 @@ static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)
reg->doneq_index = 0;
writel(ARCMSR_MESSAGE_SET_POST_WINDOW, reg->drv2iop_doorbell_reg);
if (arcmsr_hbb_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d:can not set diver mode\n", \
+ printk(KERN_NOTICE "arcmsr%d:can not set diver mode\n",
acb->host->host_no);
return 1;
}
@@ -1999,14 +1996,14 @@ static int arcmsr_iop_confirm(struct AdapterControlBlock *acb)

writel(ARCMSR_MESSAGE_SET_CONFIG, reg->drv2iop_doorbell_reg);
if (arcmsr_hbb_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: 'set command Q window' \
- timeout \n",acb->host->host_no);
+ printk(KERN_NOTICE "arcmsr%d: 'set command Q window' timeout \n",
+ acb->host->host_no);
return 1;
}

writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell_reg);
if (arcmsr_hbb_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: 'can not set diver mode \n"\
+ printk(KERN_NOTICE "arcmsr%d: 'can not set diver mode \n"
,acb->host->host_no);
return 1;
}
@@ -2048,8 +2045,8 @@ static void arcmsr_start_hba_bgrb(struct AdapterControlBlock *acb)
acb->acb_flags |= ACB_F_MSG_START_BGRB;
writel(ARCMSR_INBOUND_MESG0_START_BGRB, &reg->inbound_msgaddr0);
if (arcmsr_hba_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: wait 'start adapter background \
- rebulid' timeout \n", acb->host->host_no);
+ printk(KERN_NOTICE "arcmsr%d: wait 'start adapter background rebuild' timeout \n",
+ acb->host->host_no);
}
}

@@ -2059,8 +2056,8 @@ static void arcmsr_start_hbb_bgrb(struct AdapterControlBlock *acb)
acb->acb_flags |= ACB_F_MSG_START_BGRB;
writel(ARCMSR_MESSAGE_START_BGRB, reg->drv2iop_doorbell_reg);
if (arcmsr_hbb_wait_msgint_ready(acb)) {
- printk(KERN_NOTICE "arcmsr%d: wait 'start adapter background \
- rebulid' timeout \n",acb->host->host_no);
+ printk(KERN_NOTICE "arcmsr%d: wait 'start adapter background rebuild' timeout \n",
+ acb->host->host_no);
}
}

--
1.6.6.rc0.57.gad7a

Joe Perches

unread,
Jan 31, 2010, 3:10:04 PM1/31/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---
drivers/serial/bfin_5xx.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/serial/bfin_5xx.c b/drivers/serial/bfin_5xx.c
index 50abb7e..bea4587 100644
--- a/drivers/serial/bfin_5xx.c
+++ b/drivers/serial/bfin_5xx.c
@@ -729,8 +729,7 @@ static int bfin_serial_startup(struct uart_port *port)
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
IRQF_DISABLED, "BFIN_UART_CTS", uart)) {
uart->cts_pin = -1;
- pr_info("Unable to attach BlackFin UART CTS interrupt.\
- So, disable it.\n");
+ pr_info("Unable to attach BlackFin UART CTS interrupt. So, disable it.\n");
}
}
if (uart->rts_pin >= 0) {
@@ -742,8 +741,7 @@ static int bfin_serial_startup(struct uart_port *port)
if (request_irq(uart->status_irq,
bfin_serial_mctrl_cts_int,
IRQF_DISABLED, "BFIN_UART_MODEM_STATUS", uart)) {
- pr_info("Unable to attach BlackFin UART Modem \
- Status interrupt.\n");
+ pr_info("Unable to attach BlackFin UART Modem Status interrupt.\n");
}

if (uart->cts_pin >= 0) {
--
1.6.6.rc0.57.gad7a

Joe Perches

unread,
Jan 31, 2010, 3:40:02 PM1/31/10
to
On Sun, 2010-01-31 at 21:32 +0100, Frans Pop wrote:
> If that spacing part is really needed (is it?), wouldn't it be more
> readable as:

> > + seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu"
> > + " "
> > + "%4lu %4lu %4lu %4lu %4lu",
> > + allocs, high, grown,

If it's required (most likely not, but it's a seq_printf and
some people think those should never be modified because it's
a public interface), it should probably be explicit:

" : globalstat %7lu %6lu %5lu %4lu \t\t\t\t%4lu %4lu %4lu %4lu %4lu"

Frans Pop

unread,
Jan 31, 2010, 3:40:02 PM1/31/10
to
Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.

Fun. I'd done the same grep earlier today. AFAICT you've got all the
ones I had.

> The characters between seq_printf elements are tabs.
> That was probably not intentional, but isn't being changed.
> It's behind an #ifdef, so it could probably become a single space.
>
> Signed-off-by: Joe Perches <j...@perches.com>
> ---
> mm/slab.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 7451bda..9964619 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -4228,8 +4228,8 @@ static int s_show(struct seq_file *m, void *p)
> unsigned long node_frees = cachep->node_frees;
> unsigned long overflows = cachep->node_overflow;
>
> - seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu \
> - %4lu %4lu %4lu %4lu %4lu", allocs, high, grown,
> + seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu %4lu %4lu %4lu %4lu %4lu",
> + allocs, high, grown,
> reaped, errors, max_freeable, node_allocs,
> node_frees, overflows);
> }

If that spacing part is really needed (is it?), wouldn't it be more
readable as:


> + seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu"

> + " "
> + "%4lu %4lu %4lu %4lu %4lu",
> + allocs, high, grown,

Also, are there supposed to be tabs in that spacing part?

Cheers,
FJP

Frans Pop

unread,
Jan 31, 2010, 7:00:03 PM1/31/10
to
On Sunday 31 January 2010, Joe Perches wrote:
> On Sun, 2010-01-31 at 21:32 +0100, Frans Pop wrote:
> > If that spacing part is really needed (is it?), wouldn't it be more
> >
> > readable as:
> > > + seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu"
> > > + " "
> > > + "%4lu %4lu %4lu %4lu %4lu",
> > > + allocs, high, grown,
>
> If it's required (most likely not, but it's a seq_printf and
> some people think those should never be modified because it's
> a public interface), it should probably be explicit:
>
> " : globalstat %7lu %6lu %5lu %4lu \t\t\t\t%4lu %4lu %4lu %4lu %4lu"

Yes, would be better. And if it is kept for compatibility it probably
deserves a comment explaining the weirdness.

Cheers,
FJP

James Bottomley

unread,
Feb 1, 2010, 12:20:02 PM2/1/10
to
On Sun, 2010-01-31 at 12:02 -0800, Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.

Why? It's perfectly valid ansi C.

> Fix rebulid/rebuild typos
>
> Signed-off-by: Joe Perches <j...@perches.com>
> ---
> drivers/scsi/arcmsr/arcmsr_hba.c | 49 +++++++++++++++++--------------------
> 1 files changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 47d5d19..a0378d5 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -585,8 +585,8 @@ static void arcmsr_flush_hba_cache(struct AdapterControlBlock *acb)
> break;
> else {
> retry_count--;
> - printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' \
> - timeout, retry count down = %d \n", acb->host->host_no, retry_count);

So I might personally dislike this style

> + printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' timeout, retry count down = %d \n",
> + acb->host->host_no, retry_count);

but I also dislike your solution; I'd have split the string into two
separate ones and relied on compiler concatenation.

However, the point is that all three are perfectly legal C. Choosing
one form over another is something best left to the maintainers rather
than imposing one style by fiat.

Consider this change veto'd unless you can get an explicit ack from the
current maintainer for changing their style.

James

Joe Perches

unread,
Feb 1, 2010, 12:40:03 PM2/1/10
to
On Mon, 2010-02-01 at 11:16 -0600, James Bottomley wrote:
> On Sun, 2010-01-31 at 12:02 -0800, Joe Perches wrote:
> > String constants that are continued on subsequent lines with \
> > are not good.
>
> Why? It's perfectly valid ansi C.

Because they generally add unwanted spaces or tabs between
words in logging messages. Just like these do.

> but I also dislike your solution; I'd have split the string into two
> separate ones and relied on compiler concatenation.

Which Linus dislikes because it makes grepping difficult.

> However, the point is that all three are perfectly legal C. Choosing
> one form over another is something best left to the maintainers rather
> than imposing one style by fiat.

Which a patch does not do.

> Consider this change veto'd unless you can get an explicit ack from the
> current maintainer for changing their style.

The current messages are defective.

Frans Pop

unread,
Feb 1, 2010, 1:10:03 PM2/1/10
to
James Bottomley wrote:
>> - printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' \
>> - timeout, retry count down = %d \n", acb->host->host_no, retry_count);
>
> So I might personally dislike this style

The problem here is not style, but that the whitespace of the indentation
on the second line becomes part of the output!
That makes the code defective and is why Joe posted the patch series.

Cheers,
FJP

Frans Pop

unread,
Feb 1, 2010, 2:10:03 PM2/1/10
to
> String constants that are continued on subsequent lines with \
> are not good.
> Fix rebulid/rebuild typos
>
> Signed-off-by: Joe Perches <j...@perches.com>

Hmmm. Would it be an idea to also get rid of all the trailing spaces in the
printks, or is that for another cleanup?

If you'd prefer a separate patch, I can do it.

A few random examples:

> �drivers/scsi/arcmsr/arcmsr_hba.c | � 49


> +++++++++++++++++-------------------- 1 files changed, 23 insertions(+),
> 26 deletions(-)
>
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c index 47d5d19..a0378d5 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c

[...]


+ printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' timeout, retry count down = %d \n",
+ acb->host->host_no, retry_count);

[...]


+ printk(KERN_NOTICE "arcmsr%d: isr get an illegal ccb command done acb = '0x%p'"
"ccb = '0x%p' ccbacb = '0x%p' startdone = 0x%x"

" ccboutstandingcount = %d \n"
, acb->host->host_no
[...]
printk(KERN_NOTICE


+ "arcmsr%d: wait 'stop adapter background rebuild' timeout \n"
, acb->host->host_no);

[...]


+ printk(KERN_NOTICE "arcmsr%d: 'can not set diver mode \n"

Joe Perches

unread,
Feb 1, 2010, 2:40:04 PM2/1/10
to
On Mon, 2010-02-01 at 20:07 +0100, Frans Pop wrote:
> Would it be an idea to also get rid of all the trailing spaces in printks?

>
> If you'd prefer a separate patch, I can do it.

(cc:s trimmed)

I think it's useful, though like all of these cleanups,
a low priority. I suggest you submit a few through to
various maintainers and see what happens.

(with false positives for asm and such)
$ grep -rP --include=*.[ch] "\s+\\\n" * | wc -l
7339

There are probably a couple of thousand wasted bytes in
an allyesconfig image and rather more in various logs on
any number of systems.

Frans Pop

unread,
Feb 1, 2010, 3:20:03 PM2/1/10
to
On Monday 01 February 2010, Joe Perches wrote:
> I think it's useful, though like all of these cleanups,
> a low priority. I suggest you submit a few through to
> various maintainers and see what happens.

Will do.

> (with false positives for asm and such)
> $ grep -rP --include=*.[ch] "\s+\\\n" * | wc -l
> 7339

I'm left with 2303 after cleaning out false positives (of which 1016 in
staging...).

Cheers,
FJP

Joe Perches

unread,
Feb 1, 2010, 3:40:02 PM2/1/10
to
Signed-off-by: Joe Perches <j...@perches.com>
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3257d3d..4a4d55c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1383,6 +1383,11 @@ sub process {
WARN("line over 80 characters\n" . $herecurr);
}

+# check for spaces before a quoted newline
+ if ($rawline =~ /^.*\".*\s\\n/) {
+ WARN("unnecessary whitespace before a quoted newline\n" . $herecurr);
+ }
+
# check for adding lines without a newline.
if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) {
WARN("adding a line without newline at end of file\n" . $herecurr);

Stefan Richter

unread,
Feb 2, 2010, 1:30:02 PM2/2/10
to
Frans Pop wrote:
> James Bottomley wrote:
>>> - printk(KERN_NOTICE "arcmsr%d: wait 'flush adapter cache' \
>>> - timeout, retry count down = %d \n", acb->host->host_no, retry_count);
>>
>> So I might personally dislike this style
>
> The problem here is not style, but that the whitespace of the indentation
> on the second line becomes part of the output!
> That makes the code defective and is why Joe posted the patch series.

Joe could have pointed this out in the changelog.
s/are not good/incorporate unintended whitespace/

[James wrote:]


>> Why? It's perfectly valid ansi C.

Its syntax is valid but not its semantics.

>> Consider this change veto'd unless you can get an explicit ack from the
>> current maintainer for changing their style.

How about the maintainer takes the fix patch and adjusts style to his
liking? (That's what I would do because I like fix submissions.)
--
Stefan Richter
-=====-==-=- --=- ---=-
http://arcgraph.de/sr/

David Miller

unread,
Feb 3, 2010, 9:50:01 PM2/3/10
to
From: Joe Perches <j...@perches.com>
Date: Sun, 31 Jan 2010 12:02:09 -0800

> String constants that are continued on subsequent lines with \
> are not good.
>
> Signed-off-by: Joe Perches <j...@perches.com>

Applied, thanks.

0 new messages