[PATCH 08/10] fs/proc/array.c: Fix continuation line formats
Joe Perches
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 <j...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Joe Perches
are not good.
Signed-off-by: Joe Perches <j...@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;
Matt Mackall
> String constants that are continued on subsequent lines with \
> are not good.
> The characters between seq_printf elements are tabs.
> That was probably not intentional, but isn't being changed.
> It's behind an #ifdef, so it could probably become a single space.
>
> Signed-off-by: Joe Perches <j...@perches.com>
> ---
> mm/slab.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 7451bda..9964619 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -4228,8 +4228,8 @@ static int s_show(struct seq_file *m, void *p)
> unsigned long node_frees = cachep->node_frees;
> unsigned long overflows = cachep->node_overflow;
>
> - seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu \
> - %4lu %4lu %4lu %4lu %4lu", allocs, high, grown,
> + seq_printf(m, " : globalstat %7lu %6lu %5lu %4lu %4lu %4lu %4lu %4lu %4lu",
> + allocs, high, grown,
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
Joe Perches
are not good.
Signed-off-by: Joe Perches <j...@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
Joe Perches
are not good.
Signed-off-by: Joe Perches <j...@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;
Joe Perches
are not good.
Signed-off-by: Joe Perches <j...@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);
Joe Perches
> 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.
Gábor Stefanik
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
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
>
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
Benjamin Herrenschmidt
> are not good.
>
> Signed-off-by: Joe Perches <j...@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 :-)
--
Mark Brown
> String constants that are continued on subsequent lines with \
> are not good.
>
> Signed-off-by: Joe Perches <j...@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.
Joe Perches
> 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 <j...@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.
Joe Perches
> 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 <j...@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?
Mike Frysinger
> 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 <j...@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
Mark Brown
> 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).
Joe Perches
> 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]/) {
John Kacur
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)\
Mike Frysinger
those examples are missing spaces before the \ ;)
-mike
Joe Perches
> On Tue, Feb 2, 2010 at 1:55 PM, Joe Perches <j...@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).
> 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]/) {
Daniel Walker
> 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
Andy Whitcroft
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
David Miller
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 <j...@perches.com>
Applied, thanks.
Benjamin Herrenschmidt
> 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 <j...@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.