[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 01/10] arch/powerpc: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <joe@perches.com>
---
 arch/powerpc/kernel/nvram_64.c               |    6 +++---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |    8 ++++----
 arch/powerpc/platforms/pseries/smp.c         |    4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index ad461e7..9cf197f 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -338,8 +338,8 @@ static int __init nvram_create_os_partition(void)
 
 	rc = nvram_write_header(new_part);
 	if (rc <= 0) {
-		printk(KERN_ERR "nvram_create_os_partition: nvram_write_header \
-				failed (%d)\n", rc);
+		printk(KERN_ERR "nvram_create_os_partition: nvram_write_header "
+				"failed (%d)\n", rc);
 		return rc;
 	}
 
@@ -349,7 +349,7 @@ static int __init nvram_create_os_partition(void)
 	rc = ppc_md.nvram_write((char *)&seq_init, sizeof(seq_init), &tmp_index);
 	if (rc <= 0) {
 		printk(KERN_ERR "nvram_create_os_partition: nvram_write "
-				"failed (%d)\n", rc);
+		       "failed (%d)\n", rc);
 		return rc;
 	}
 	
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 6ea4698..a70de10 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -397,12 +397,12 @@ static int parse_cede_parameters(void)
 				CEDE_LATENCY_PARAM_MAX_LENGTH);
 
 	if (call_status != 0)
-		printk(KERN_INFO "CEDE_LATENCY: \
-			%s %s Error calling get-system-parameter(0x%x)\n",
+		printk(KERN_INFO "CEDE_LATENCY: "
+			"%s %s Error calling get-system-parameter(0x%x)\n",
 			__FILE__, __func__, call_status);
 	else
-		printk(KERN_INFO "CEDE_LATENCY: \
-			get-system-parameter successful.\n");
+		printk(KERN_INFO "CEDE_LATENCY: "
+			"get-system-parameter successful.\n");
 
 	return call_status;
 }
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index b488663..4e7f89a 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -144,8 +144,8 @@ static void __devinit smp_pSeries_kick_cpu(int nr)
 		hcpuid = get_hard_smp_processor_id(nr);
 		rc = plpar_hcall_norets(H_PROD, hcpuid);
 		if (rc != H_SUCCESS)
-			printk(KERN_ERR "Error: Prod to wake up processor %d\
-						Ret= %ld\n", nr, rc);
+			printk(KERN_ERR "Error: Prod to wake up processor %d "
+						"Ret= %ld\n", nr, rc);
 	}
 }
 
-- 
1.6.6.rc0.57.gad7a

Re: [PATCH 01/10] arch/powerpc: Fix continuation line formats

[Benjamin Herrenschmidt]

On Sun, 2010-01-31 at 12:02 -0800, Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

You want me to take that in the powerpc tree ?

A minor glitch below tho...

> ---
>  arch/powerpc/kernel/nvram_64.c               |    6 +++---
>  arch/powerpc/platforms/pseries/hotplug-cpu.c |    8 ++++----
>  arch/powerpc/platforms/pseries/smp.c         |    4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index ad461e7..9cf197f 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -338,8 +338,8 @@ static int __init nvram_create_os_partition(void)
>  
>  	rc = nvram_write_header(new_part);
>  	if (rc <= 0) {
> -		printk(KERN_ERR "nvram_create_os_partition: nvram_write_header \
> -				failed (%d)\n", rc);
> +		printk(KERN_ERR "nvram_create_os_partition: nvram_write_header "
> +				"failed (%d)\n", rc);
>  		return rc;
>  	}
>  
> @@ -349,7 +349,7 @@ static int __init nvram_create_os_partition(void)
>  	rc = ppc_md.nvram_write((char *)&seq_init, sizeof(seq_init), &tmp_index);
>  	if (rc <= 0) {
>  		printk(KERN_ERR "nvram_create_os_partition: nvram_write "
> -				"failed (%d)\n", rc);
> +		       "failed (%d)\n", rc);
>  		return rc;
>  	}

The above is objectionable :-)

>  	
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 6ea4698..a70de10 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -397,12 +397,12 @@ static int parse_cede_parameters(void)
>  				CEDE_LATENCY_PARAM_MAX_LENGTH);
>  
>  	if (call_status != 0)
> -		printk(KERN_INFO "CEDE_LATENCY: \
> -			%s %s Error calling get-system-parameter(0x%x)\n",
> +		printk(KERN_INFO "CEDE_LATENCY: "
> +			"%s %s Error calling get-system-parameter(0x%x)\n",
>  			__FILE__, __func__, call_status);
>  	else
> -		printk(KERN_INFO "CEDE_LATENCY: \
> -			get-system-parameter successful.\n");
> +		printk(KERN_INFO "CEDE_LATENCY: "
> +			"get-system-parameter successful.\n");
>  
>  	return call_status;
>  }
> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
> index b488663..4e7f89a 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -144,8 +144,8 @@ static void __devinit smp_pSeries_kick_cpu(int nr)
>  		hcpuid = get_hard_smp_processor_id(nr);
>  		rc = plpar_hcall_norets(H_PROD, hcpuid);
>  		if (rc != H_SUCCESS)
> -			printk(KERN_ERR "Error: Prod to wake up processor %d\
> -						Ret= %ld\n", nr, rc);
> +			printk(KERN_ERR "Error: Prod to wake up processor %d "
> +						"Ret= %ld\n", nr, rc);
>  	}
>  }
>  

Re: [PATCH 01/10] arch/powerpc: Fix continuation line formats

[Joe Perches]

On Mon, 2010-02-01 at 13:16 +1100, Benjamin Herrenschmidt 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.
> > Signed-off-by: Joe Perches <joe@perches.com>
> You want me to take that in the powerpc tree ?

Yes please.

> A minor glitch below tho...
> > @@ -349,7 +349,7 @@ static int __init nvram_create_os_partition(void)
> >  	rc = ppc_md.nvram_write((char *)&seq_init, sizeof(seq_init), &tmp_index);
> >  	if (rc <= 0) {
> >  		printk(KERN_ERR "nvram_create_os_partition: nvram_write "
> > -				"failed (%d)\n", rc);
> > +		       "failed (%d)\n", rc);
> >  		return rc;
> >  	}
> 
> The above is objectionable :-)

Can you drop that section or do you need another patch?

Re: [PATCH 01/10] arch/powerpc: Fix continuation line formats

[Benjamin Herrenschmidt]

On Mon, 2010-02-01 at 10:30 -0800, Joe Perches wrote:
> On Mon, 2010-02-01 at 13:16 +1100, Benjamin Herrenschmidt 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.
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > You want me to take that in the powerpc tree ?
> 
> Yes please.
> 
> > A minor glitch below tho...
> > > @@ -349,7 +349,7 @@ static int __init nvram_create_os_partition(void)
> > >  	rc = ppc_md.nvram_write((char *)&seq_init, sizeof(seq_init), &tmp_index);
> > >  	if (rc <= 0) {
> > >  		printk(KERN_ERR "nvram_create_os_partition: nvram_write "
> > > -				"failed (%d)\n", rc);
> > > +		       "failed (%d)\n", rc);
> > >  		return rc;
> > >  	}
> > 
> > The above is objectionable :-)
> 
> Can you drop that section or do you need another patch?

I'll sort it out. Thanks.

Cheers,
Ben.

[PATCH 02/10] arch/blackfin: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

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

[PATCH 03/10] drivers/ide: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/ide/au1xxx-ide.c |    4 ++--
 drivers/ide/pmac.c       |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ide/au1xxx-ide.c b/drivers/ide/au1xxx-ide.c
index 87cef0c..4beb296 100644
--- a/drivers/ide/au1xxx-ide.c
+++ b/drivers/ide/au1xxx-ide.c
@@ -300,8 +300,8 @@ static int auide_dma_test_irq(ide_drive_t *drive)
 	 */
 	drive->waiting_for_dma++;
 	if (drive->waiting_for_dma >= DMA_WAIT_TIMEOUT) {
-		printk(KERN_WARNING "%s: timeout waiting for ddma to \
-                                     complete\n", drive->name);
+		printk(KERN_WARNING "%s: timeout waiting for ddma to complete\n",
+		       drive->name);
 		return 1;
 	}
 	udelay(10);
diff --git a/drivers/ide/pmac.c b/drivers/ide/pmac.c
index 7a4e788..081e3b9 100644
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -1651,8 +1651,8 @@ pmac_ide_dma_test_irq (ide_drive_t *drive)
 		if ((status & FLUSH) == 0)
 			break;
 		if (++timeout > 100) {
-			printk(KERN_WARNING "ide%d, ide_dma_test_irq \
-			timeout flushing channel\n", hwif->index);
+			printk(KERN_WARNING "ide%d, ide_dma_test_irq timeout flushing channel\n",
+			       hwif->index);
 			break;
 		}
 	}	
-- 
1.6.6.rc0.57.gad7a

Re: [PATCH 03/10] drivers/ide: Fix continuation line formats

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Sun, 31 Jan 2010 12:02:05 -0800

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

Applied, thanks.

[PATCH 04/10] drivers/serial/bfin_5xx.c: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

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

[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

[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/

[PATCH 06/10] drivers/staging: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.
Fixed a "is tryied" / tried typo

Signed-off-by: Joe Perches <joe@perches.com>
---
 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 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/dream/qdsp5/audio_mp3.c b/drivers/staging/dream/qdsp5/audio_mp3.c
index b95574f..7ed6e26 100644
--- a/drivers/staging/dream/qdsp5/audio_mp3.c
+++ b/drivers/staging/dream/qdsp5/audio_mp3.c
@@ -650,8 +650,7 @@ static long audio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 						       &audio->read_phys,
 						       GFP_KERNEL);
 				if (!audio->read_data) {
-					pr_err("audio_mp3: malloc pcm \
-					buf failed\n");
+					pr_err("audio_mp3: malloc pcm buf failed\n");
 					rc = -1;
 				} else {
 					uint8_t index;
diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c
index f1d6cb4..ad42bcd 100644
--- a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c
+++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c
@@ -482,8 +482,7 @@ int ieee80211_wx_set_power(struct ieee80211_device *ieee,
 		(!ieee->enter_sleep_state) ||
 		(!ieee->ps_is_queue_empty)){
 
-		printk("ERROR. PS mode is tryied to be use but\
-driver missed a callback\n\n");
+		printk("ERROR. PS mode tried to be use but driver missed a callback\n\n");
 
 		return -1;
 	}
diff --git a/drivers/staging/rtl8187se/r8180_core.c b/drivers/staging/rtl8187se/r8180_core.c
index e0f13ef..278992a 100644
--- a/drivers/staging/rtl8187se/r8180_core.c
+++ b/drivers/staging/rtl8187se/r8180_core.c
@@ -3864,8 +3864,7 @@ static int __init rtl8180_pci_module_init(void)
 		return ret;
 	}
 
-	printk(KERN_INFO "\nLinux kernel driver for RTL8180 \
-/ RTL8185 based WLAN cards\n");
+	printk(KERN_INFO "\nLinux kernel driver for RTL8180 / RTL8185 based WLAN cards\n");
 	printk(KERN_INFO "Copyright (c) 2004-2005, Andrea Merello\n");
 	DMESG("Initializing module");
 	DMESG("Wireless extensions version %d", WIRELESS_EXT);
diff --git a/drivers/staging/sep/sep_driver.c b/drivers/staging/sep/sep_driver.c
index e7bc9ec..8793878 100644
--- a/drivers/staging/sep/sep_driver.c
+++ b/drivers/staging/sep/sep_driver.c
@@ -380,8 +380,7 @@ static int sep_mmap(struct file *filp, struct vm_area_struct *vma)
 	   shared area */
 	if ((vma->vm_end - vma->vm_start) > SEP_DRIVER_MMMAP_AREA_SIZE) {
 		edbg("SEP Driver mmap requested size is more than allowed\n");
-		printk(KERN_WARNING "SEP Driver mmap requested size is more \
-			than allowed\n");
+		printk(KERN_WARNING "SEP Driver mmap requested size is more than allowed\n");
 		printk(KERN_WARNING "SEP Driver vma->vm_end is %08lx\n", vma->vm_end);
 		printk(KERN_WARNING "SEP Driver vma->vm_end is %08lx\n", vma->vm_start);
 		return -EAGAIN;
-- 
1.6.6.rc0.57.gad7a

Re: [PATCH 06/10] drivers/staging: Fix continuation line formats

[Gábor Stefanik]

On Sun, Jan 31, 2010 at 9:02 PM, Joe Perches <joe@perches.com> wrote:
> String constants that are continued on subsequent lines with \
> are not good.
> Fixed a "is tryied" / tried typo
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  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 +--
>  4 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/dream/qdsp5/audio_mp3.c b/drivers/staging/dream/qdsp5/audio_mp3.c
> index b95574f..7ed6e26 100644
> --- a/drivers/staging/dream/qdsp5/audio_mp3.c
> +++ b/drivers/staging/dream/qdsp5/audio_mp3.c
> @@ -650,8 +650,7 @@ static long audio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>                                                       &audio->read_phys,
>                                                       GFP_KERNEL);
>                                if (!audio->read_data) {
> -                                       pr_err("audio_mp3: malloc pcm \
> -                                       buf failed\n");
> +                                       pr_err("audio_mp3: malloc pcm buf failed\n");
>                                        rc = -1;
>                                } else {
>                                        uint8_t index;
> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c
> index f1d6cb4..ad42bcd 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_softmac_wx.c
> @@ -482,8 +482,7 @@ int ieee80211_wx_set_power(struct ieee80211_device *ieee,
>                (!ieee->enter_sleep_state) ||
>                (!ieee->ps_is_queue_empty)){
>
> -               printk("ERROR. PS mode is tryied to be use but\
> -driver missed a callback\n\n");
> +               printk("ERROR. PS mode tried to be use but driver missed a callback\n\n");

This have more typos... "to be use"?

>
>                return -1;
>        }
> diff --git a/drivers/staging/rtl8187se/r8180_core.c b/drivers/staging/rtl8187se/r8180_core.c
> index e0f13ef..278992a 100644
> --- a/drivers/staging/rtl8187se/r8180_core.c
> +++ b/drivers/staging/rtl8187se/r8180_core.c
> @@ -3864,8 +3864,7 @@ static int __init rtl8180_pci_module_init(void)
>                return ret;
>        }
>
> -       printk(KERN_INFO "\nLinux kernel driver for RTL8180 \
> -/ RTL8185 based WLAN cards\n");
> +       printk(KERN_INFO "\nLinux kernel driver for RTL8180 / RTL8185 based WLAN cards\n");

While you are at it, change this to read RTL8187SE, since RTL8180/85
are not supported by this driver.

>        printk(KERN_INFO "Copyright (c) 2004-2005, Andrea Merello\n");
>        DMESG("Initializing module");
>        DMESG("Wireless extensions version %d", WIRELESS_EXT);
> diff --git a/drivers/staging/sep/sep_driver.c b/drivers/staging/sep/sep_driver.c
> index e7bc9ec..8793878 100644
> --- a/drivers/staging/sep/sep_driver.c
> +++ b/drivers/staging/sep/sep_driver.c
> @@ -380,8 +380,7 @@ static int sep_mmap(struct file *filp, struct vm_area_struct *vma)
>           shared area */
>        if ((vma->vm_end - vma->vm_start) > SEP_DRIVER_MMMAP_AREA_SIZE) {
>                edbg("SEP Driver mmap requested size is more than allowed\n");
> -               printk(KERN_WARNING "SEP Driver mmap requested size is more \
> -                       than allowed\n");
> +               printk(KERN_WARNING "SEP Driver mmap requested size is more than allowed\n");
>                printk(KERN_WARNING "SEP Driver vma->vm_end is %08lx\n", vma->vm_end);
>                printk(KERN_WARNING "SEP Driver vma->vm_end is %08lx\n", vma->vm_start);
>                return -EAGAIN;
> --
> 1.6.6.rc0.57.gad7a
>
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

[PATCH 07/10] drivers/net/amd8111e.c: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

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

Re: [PATCH 07/10] drivers/net/amd8111e.c: Fix continuation line formats

[David Miller]

From: Joe Perches <joe@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 <joe@perches.com>

Applied, thanks.

[PATCH 08/10] fs/proc/array.c: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

The character at the end of the 3rd line is a tab not a space.
That was probably not intentional, but I'm not changing it.

Signed-off-by: Joe Perches <joe@perches.com>
---
 fs/proc/array.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 13b5d07..0b4c4ce 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -465,9 +465,10 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	/* convert nsec -> ticks */
 	start_time = nsec_to_clock_t(start_time);
 
-	seq_printf(m, "%d (%s) %c %d %d %d %d %d %u %lu \
-%lu %lu %lu %lu %lu %ld %ld %ld %ld %d 0 %llu %lu %ld %lu %lu %lu %lu %lu \
-%lu %lu %lu %lu %lu %lu %lu %lu %d %d %u %u %llu %lu %ld\n",
+	seq_printf(m, "%d (%s) %c %d %d %d %d %d %u %lu "
+		   "%lu %lu %lu %lu %lu %ld %ld %ld %ld "
+		   "%d 0 %llu %lu %ld %lu %lu %lu %lu %lu	"
+		   "%lu %lu %lu %lu %lu %lu %lu %lu %d %d %u %u %llu %lu %ld\n",
 		pid_nr_ns(pid, ns),
 		tcomm,
 		state,
-- 
1.6.6.rc0.57.gad7a

[PATCH 09/10] mm/slab.c: Fix continuation line formats

[Joe Perches]

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

Re: [PATCH 09/10] mm/slab.c: Fix continuation line formats

[Matt Mackall]

On Sun, 2010-01-31 at 12:02 -0800, Joe Perches wrote:
> 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 <joe@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,

Yuck. The right way to do this is by mergeable adjacent strings, eg:

printk("part 1..."
       " part 2...", ...);

-- 
http://selenic.com : development and support for Mercurial and Linux

Re: [PATCH 09/10] mm/slab.c: Fix continuation line formats

[Joe Perches]

On Sun, 2010-01-31 at 14:08 -0600, Matt Mackall wrote:
> On Sun, 2010-01-31 at 12:02 -0800, Joe Perches wrote:
> > 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,
> 
> Yuck. The right way to do this is by mergeable adjacent strings, eg:
> 
> printk("part 1..."
>        " part 2...", ...);

Yuck indeed.

I think format strings shouldn't be split across multiple lines and
the right thing to do is to use a single space instead of the tabs.

Re: [PATCH 09/10] mm/slab.c: Fix continuation line formats

[Frans Pop]

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

Re: [PATCH 09/10] mm/slab.c: Fix continuation line formats

[Joe Perches]

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"

Re: [PATCH 09/10] mm/slab.c: Fix continuation line formats

[Frans Pop]

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

[PATCH 10/10] sound/soc/blackfin: Fix continuation line formats

[Joe Perches]

String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <joe@perches.com>
---
 sound/soc/blackfin/bf5xx-ac97-pcm.c |    8 ++------
 sound/soc/blackfin/bf5xx-i2s-pcm.c  |    3 +--
 sound/soc/blackfin/bf5xx-tdm-pcm.c  |    3 +--
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c
index cf0dfb7..67cbfe7 100644
--- a/sound/soc/blackfin/bf5xx-ac97-pcm.c
+++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c
@@ -349,9 +349,7 @@ static int bf5xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
 			sport_handle->tx_dma_buf = dma_alloc_coherent(NULL, \
 				size, &sport_handle->tx_dma_phy, GFP_KERNEL);
 			if (!sport_handle->tx_dma_buf) {
-				pr_err("Failed to allocate memory for tx dma \
-					buf - Please increase uncached DMA \
-					memory region\n");
+				pr_err("Failed to allocate memory for tx dma buf - Please increase uncached DMA memory region\n");
 				return -ENOMEM;
 			} else
 				memset(sport_handle->tx_dma_buf, 0, size);
@@ -362,9 +360,7 @@ static int bf5xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
 			sport_handle->rx_dma_buf = dma_alloc_coherent(NULL, \
 				size, &sport_handle->rx_dma_phy, GFP_KERNEL);
 			if (!sport_handle->rx_dma_buf) {
-				pr_err("Failed to allocate memory for rx dma \
-					buf - Please increase uncached DMA \
-					memory region\n");
+				pr_err("Failed to allocate memory for rx dma buf - Please increase uncached DMA memory region\n");
 				return -ENOMEM;
 			} else
 				memset(sport_handle->rx_dma_buf, 0, size);
diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c
index 62fbb84..c6c6a4a 100644
--- a/sound/soc/blackfin/bf5xx-i2s-pcm.c
+++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c
@@ -207,8 +207,7 @@ static int bf5xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
 	buf->area = dma_alloc_coherent(pcm->card->dev, size,
 			&buf->addr, GFP_KERNEL);
 	if (!buf->area) {
-		pr_err("Failed to allocate dma memory \
-			Please increase uncached DMA memory region\n");
+		pr_err("Failed to allocate dma memory - Please increase uncached DMA memory region\n");
 		return -ENOMEM;
 	}
 	buf->bytes = size;
diff --git a/sound/soc/blackfin/bf5xx-tdm-pcm.c b/sound/soc/blackfin/bf5xx-tdm-pcm.c
index a8c73cb..5e03bb2 100644
--- a/sound/soc/blackfin/bf5xx-tdm-pcm.c
+++ b/sound/soc/blackfin/bf5xx-tdm-pcm.c
@@ -244,8 +244,7 @@ static int bf5xx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
 	buf->area = dma_alloc_coherent(pcm->card->dev, size * 4,
 		&buf->addr, GFP_KERNEL);
 	if (!buf->area) {
-		pr_err("Failed to allocate dma memory \
-			Please increase uncached DMA memory region\n");
+		pr_err("Failed to allocate dma memory - Please increase uncached DMA memory region\n");
 		return -ENOMEM;
 	}
 	buf->bytes = size;
-- 
1.6.6.rc0.57.gad7a

Re: [PATCH 10/10] sound/soc/blackfin: Fix continuation line formats

[Mark Brown]

On Sun, Jan 31, 2010 at 12:02:12PM -0800, Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Gah, I thought I'd caught most of these when reviewing.  If you're using
a script to pick this stuff up it'd be worth checking for extraneous
continuations in the middle of code - outside of macros there's little
call for it.

Applied, thanks.

Re: [PATCH 10/10] sound/soc/blackfin: Fix continuation line formats

[Joe Perches]

On Mon, 2010-02-01 at 14:47 +0000, Mark Brown wrote:
> On Sun, Jan 31, 2010 at 12:02:12PM -0800, Joe Perches wrote:
> > String constants that are continued on subsequent lines with \
> > are not good.
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> Gah, I thought I'd caught most of these when reviewing.  If you're using
> a script to pick this stuff up it'd be worth checking for extraneous
> continuations in the middle of code - outside of macros there's little
> call for it.
> 
> Applied, thanks.

There are a few false positives and probably a few missing using

        grep -rP --include=*.[ch] '".*\\$' * | \
        awk '{ if ((gsub("\"", "\"") % 2) == 1) print $0; }'
        
Most of the uses are __asm__ __volatile__ which could be
considered unsightly but don't impact logging messages.

The rest could/should be fixed.

Re: [PATCH 10/10] sound/soc/blackfin: Fix continuation line formats

[Mike Frysinger]

On Mon, Feb 1, 2010 at 12:08, Joe Perches wrote:
> On Mon, 2010-02-01 at 14:47 +0000, Mark Brown wrote:
>> On Sun, Jan 31, 2010 at 12:02:12PM -0800, Joe Perches wrote:
>> > String constants that are continued on subsequent lines with \
>> > are not good.
>> >
>> > Signed-off-by: Joe Perches <joe@perches.com>
>>
>> Gah, I thought I'd caught most of these when reviewing.  If you're using
>> a script to pick this stuff up it'd be worth checking for extraneous
>> continuations in the middle of code - outside of macros there's little
>> call for it.
>>
>> Applied, thanks.
>
> There are a few false positives and probably a few missing using
>
>        grep -rP --include=*.[ch] '".*\\$' * | \
>        awk '{ if ((gsub("\"", "\"") % 2) == 1) print $0; }'
>
> Most of the uses are __asm__ __volatile__ which could be
> considered unsightly but don't impact logging messages.
>
> The rest could/should be fixed.

*cough* checkpatch.pl *cough*

the Blackfin alsa fixes all look good to me, thanks
-mike

Re: [PATCH 10/10] sound/soc/blackfin: Fix continuation line formats

[Mark Brown]

On Mon, Feb 01, 2010 at 11:13:04PM -0500, Mike Frysinger wrote:
> On Mon, Feb 1, 2010 at 12:08, Joe Perches wrote:

> > There are a few false positives and probably a few missing using

> >        grep -rP --include=*.[ch] '".*\\$' * | \
> >        awk '{ if ((gsub("\"", "\"") % 2) == 1) print $0; }'

> > Most of the uses are __asm__ __volatile__ which could be
> > considered unsightly but don't impact logging messages.

> > The rest could/should be fixed.

My point was that it'd be good to also check for just regular use of
continuations in code other than macro definitions.  These are just a
style nit but if there's a script that filters out false positives from
the macros that'd be handy...

> the Blackfin alsa fixes all look good to me, thanks

Running "grep ' \\$' sound/soc/blackfin/*.[ch]" suggests that there's
still some of the continuations I mentioned above in there (plus a lot
of false positives from macros).

[RFC PATCH] checkpatch.pl: Add warning on non #define continuation lines

[Joe Perches]

On Tue, 2010-02-02 at 11:08 +0000, Mark Brown wrote:
> it'd be good to also check for just regular use of
> continuations in code other than macro definitions.  These are just a
> style nit but if there's a script that filters out false positives from
> the macros that'd be handy...

> Running "grep ' \\$' sound/soc/blackfin/*.[ch]" suggests that there's
> still some of the continuations I mentioned above in there (plus a lot
> of false positives from macros).

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3257d3d..a174501 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1234,6 +1234,7 @@ sub process {
 
 	$realcnt = 0;
 	$linenr = 0;
+	my $in_define = 0;
 	foreach my $line (@lines) {
 		$linenr++;
 
@@ -1388,6 +1389,16 @@ sub process {
 			WARN("adding a line without newline at end of file\n" . $herecurr);
 		}
 
+		if ($rawline =~ /\\$/) {
+		    if ($rawline =~ /\s*\#\s*define\s+/) {
+			$in_define = 1;
+		    } elsif (!$in_define) {
+			WARN("Continuations outside of macros should be avoided\n" . $herecurr);
+		    }
+		} else {
+		    $in_define = 0;
+		}
+
 # Blackfin: use hi/lo macros
 		if ($realfile =~ m@arch/blackfin/.*\.S$@) {
 			if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) {

Re: [RFC PATCH] checkpatch.pl: Add warning on non #define continuation lines

[John Kacur]

On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2010-02-02 at 11:08 +0000, Mark Brown wrote:
>> it'd be good to also check for just regular use of
>> continuations in code other than macro definitions.  These are just a
>> style nit but if there's a script that filters out false positives from
>> the macros that'd be handy...
>
>> Running "grep ' \\$' sound/soc/blackfin/*.[ch]" suggests that there's
>> still some of the continuations I mentioned above in there (plus a lot
>> of false positives from macros).
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3257d3d..a174501 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1234,6 +1234,7 @@ sub process {
>
>        $realcnt = 0;
>        $linenr = 0;
> +       my $in_define = 0;
>        foreach my $line (@lines) {
>                $linenr++;
>
> @@ -1388,6 +1389,16 @@ sub process {
>                        WARN("adding a line without newline at end of file\n" . $herecurr);
>                }
>
> +               if ($rawline =~ /\\$/) {
> +                   if ($rawline =~ /\s*\#\s*define\s+/) {
> +                       $in_define = 1;
> +                   } elsif (!$in_define) {
> +                       WARN("Continuations outside of macros should be avoided\n" . $herecurr);
> +                   }
> +               } else {
> +                   $in_define = 0;
> +               }
> +
>  # Blackfin: use hi/lo macros
>                if ($realfile =~ m@arch/blackfin/.*\.S$@) {
>                        if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) {
>
>find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3 Continuations

Checkpatch is already kind of loud, so I'm not sure I like the idea -
you even say yourself that it's just a style nit.
But just in-case there are a lot of people who do like this, then you
have to do some work to get rid of false positives. Try something like
the following.

1. apply your patch.
2. run something like
find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3
Continuations

and then go through the results looking for false positives.

One that I see right away, is with #if
For example,

#39: FILE: arch/arm/mach-lh7a40x/lcd-panel.h:39:
+#if defined (MACH_LPD79520)\

WARNING: Continuations outside of macros should be avoided
#40: FILE: arch/arm/mach-lh7a40x/lcd-panel.h:40:
+ || defined (MACH_LPD79524)\

Re: [RFC PATCH] checkpatch.pl: Add warning on non #define continuation lines

[Mike Frysinger]

On Tue, Feb 2, 2010 at 08:49, John Kacur wrote:
> On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches wrote:
>> On Tue, 2010-02-02 at 11:08 +0000, Mark Brown wrote:
>>> it'd be good to also check for just regular use of
>>> continuations in code other than macro definitions.  These are just a
>>> style nit but if there's a script that filters out false positives from
>>> the macros that'd be handy...
>>
>>> Running "grep ' \\$' sound/soc/blackfin/*.[ch]" suggests that there's
>>> still some of the continuations I mentioned above in there (plus a lot
>>> of false positives from macros).
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 3257d3d..a174501 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1234,6 +1234,7 @@ sub process {
>>
>>        $realcnt = 0;
>>        $linenr = 0;
>> +       my $in_define = 0;
>>        foreach my $line (@lines) {
>>                $linenr++;
>>
>> @@ -1388,6 +1389,16 @@ sub process {
>>                        WARN("adding a line without newline at end of file\n" . $herecurr);
>>                }
>>
>> +               if ($rawline =~ /\\$/) {
>> +                   if ($rawline =~ /\s*\#\s*define\s+/) {
>> +                       $in_define = 1;
>> +                   } elsif (!$in_define) {
>> +                       WARN("Continuations outside of macros should be avoided\n" . $herecurr);
>> +                   }
>> +               } else {
>> +                   $in_define = 0;
>> +               }
>> +
>>  # Blackfin: use hi/lo macros
>>                if ($realfile =~ m@arch/blackfin/.*\.S$@) {
>>                        if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) {
>>
>>find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3 Continuations
>
> Checkpatch is already kind of loud, so I'm not sure I like the idea -
> you even say yourself that it's just a style nit.
> But just in-case there are a lot of people who do like this, then you
> have to do some work to get rid of false positives. Try something like
> the following.
>
> 1. apply your patch.
> 2. run something like
> find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3
> Continuations
>
> and then go through the results looking for false positives.
>
> One that I see right away, is with #if
> For example,
>
> #39: FILE: arch/arm/mach-lh7a40x/lcd-panel.h:39:
> +#if defined (MACH_LPD79520)\
>
> WARNING: Continuations outside of macros should be avoided
> #40: FILE: arch/arm/mach-lh7a40x/lcd-panel.h:40:
> + || defined (MACH_LPD79524)\

those examples are missing spaces before the \ ;)
-mike

[RFC PATCH V2] checkpatch.pl: Add warning on non #define continuation lines

[Joe Perches]

On Tue, 2010-02-02 at 14:49 +0100, John Kacur wrote:
> On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches <joe@perches.com> wrote:
> > On Tue, 2010-02-02 at 11:08 +0000, Mark Brown wrote:
> >> it'd be good to also check for just regular use of
> >> continuations in code other than macro definitions.  These are just a
> >> style nit but if there's a script that filters out false positives from
> >> the macros that'd be handy...
> >
> >> Running "grep ' \\$' sound/soc/blackfin/*.[ch]" suggests that there's
> >> still some of the continuations I mentioned above in there (plus a lot
> >> of false positives from macros).
> Checkpatch is already kind of loud, so I'm not sure I like the idea -
> you even say yourself that it's just a style nit.
> But just in-case there are a lot of people who do like this, then you
> have to do some work to get rid of false positives. Try something like
> the following.
> 
> 1. apply your patch.
> 2. run something like
> find ./ -name "*.[ch]" | xargs ./scripts/checkpatch.pl -f | grep -a3
> Continuations
> 
> and then go through the results looking for false positives.

This is better.

There's probably something more appropriate that Andy Whitcroft
could work out that actually apply patches and verifies the
code using something like the ctx_statement_block functions but
that code is overly mysterious to me.

Does anyone know if Andy Whitcroft is still looking after checkpatch?

---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3257d3d..cc5d8ee 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1234,6 +1234,7 @@ sub process {
 
 	$realcnt = 0;
 	$linenr = 0;
+	my $in_define = 0;
 	foreach my $line (@lines) {
 		$linenr++;
 
@@ -1388,6 +1389,17 @@ sub process {
 			WARN("adding a line without newline at end of file\n" . $herecurr);
 		}
 
+# check for line continuations that are not macros or defines
+		if ($rawline =~ /\\$/) {
+		    if ($rawline =~ /\s*\#\s*(define|if)/) {
+			$in_define = 1;
+		    } elsif (!$in_define) {
+			WARN("Line continuations should be avoided unless in #define or #if blocks\n" . $herecurr);
+		    }
+		} else {
+		    $in_define = 0;
+		}
+
 # Blackfin: use hi/lo macros
 		if ($realfile =~ m@arch/blackfin/.*\.S$@) {
 			if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) {

Re: [RFC PATCH V2] checkpatch.pl: Add warning on non #define continuation lines

[Daniel Walker]

On Tue, 2010-02-02 at 14:34 -0800, Joe Perches wrote:

> There's probably something more appropriate that Andy Whitcroft
> could work out that actually apply patches and verifies the
> code using something like the ctx_statement_block functions but
> that code is overly mysterious to me.
> 
> Does anyone know if Andy Whitcroft is still looking after checkpatch?

He's often slow to respond ..

Daniel

Re: [RFC PATCH V2] checkpatch.pl: Add warning on non #define continuation lines

[Andy Whitcroft]

> Does anyone know if Andy Whitcroft is still looking after checkpatch?

Yeah I am still about, just a lot behind on all things checkpatch.  I am
going to be on a plane in the next couple of days and thats a good time
to get to the checkpatch backlog.

-apw

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"

[RFC] Fix unnecessary spaces before newlines in logging messages

[Joe Perches]

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.

Re: [RFC] Fix unnecessary spaces before newlines in logging messages

[Frans Pop]

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

[PATCH] Warn on unnecessary spaces before quoted newlines

[Joe Perches]

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