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(-)
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <[email protected]>
---
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
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 <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <[email protected]>
---
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
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 <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Fixed a "is tryied" / tried typo
Signed-off-by: Joe Perches <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Signed-off-by: Joe Perches <[email protected]>
---
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
String constants that are continued on subsequent lines with \
are not good.
Fix rebulid/rebuild typos
Signed-off-by: Joe Perches <[email protected]>
---
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, ®->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, \
®->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, ®->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
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 <[email protected]>
> ---
> 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
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.
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 <[email protected]>
> ---
> 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
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"
On Sun, Jan 31, 2010 at 9:02 PM, Joe Perches <[email protected]> wrote:
> String constants that are continued on subsequent lines with \
> are not good.
> Fixed a "is tryied" / tried typo
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> ?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
> [email protected]
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
>
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
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
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 <[email protected]>
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);
> }
> }
>
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 <[email protected]>
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.
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 <[email protected]>
>
> 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.
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 <[email protected]>
> ---
> 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
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.
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
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 <[email protected]>
> 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?
> String constants that are continued on subsequent lines with \
> are not good.
> Fix rebulid/rebuild typos
>
> Signed-off-by: Joe Perches <[email protected]>
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"
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.
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
Signed-off-by: Joe Perches <[email protected]>
---
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);
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 <[email protected]>
>>
>> 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
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).
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]/) {
On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches <[email protected]> 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)\
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
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/
On Tue, 2010-02-02 at 14:49 +0100, John Kacur wrote:
> On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches <[email protected]> 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]/) {
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
> 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
From: Joe Perches <[email protected]>
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 <[email protected]>
Applied, thanks.
From: Joe Perches <[email protected]>
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 <[email protected]>
Applied, thanks.
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 <[email protected]>
> > 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.