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

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

[Joe Perches]

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(-)

[PATCH 05/10] drivers/scsi/arcmsr: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.
Fix rebulid/rebuild typos

Signed-off-by: Joe Perches <joe@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

Re: [PATCH 05/10] drivers/scsi/arcmsr: Fix continuation line formats

[James Bottomley]

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 <joe@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

Re: [PATCH 05/10] drivers/scsi/arcmsr: Fix continuation line formats

[Joe Perches]

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.

Re: [PATCH 05/10] drivers/scsi/arcmsr: Fix continuation line formats

[Frans Pop]

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

Re: [PATCH 05/10] drivers/scsi/arcmsr: Fix continuation line formats

[Frans Pop]

> String constants that are continued on subsequent lines with \
> are not good.
> Fix rebulid/rebuild typos
>
> Signed-off-by: Joe Perches <joe@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"

Re: [PATCH 05/10] drivers/scsi/arcmsr: Fix continuation line formats

[Stefan Richter]

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/