* [PATCH 00/39] Cleanup WARN #defines @ 2010-10-30 21:08 Joe Perches 2010-10-30 21:08 ` [PATCH 02/39] arch/alpha: Update WARN uses Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2010-10-30 21:08 UTC (permalink / raw) To: Jiri Kosina Cc: linux-alpha, linux-kernel, linux-arm-kernel, linux-omap, linux-tegra, linuxppc-dev, linux-pm, discuss, linux-acpi, linux1394-devel, dri-devel, intel-gfx, linux-media, socketcan-core, netdev, linux-usb, linux-wireless, devel, linux-scsi, devel, linux-fbdev, linux-fsdevel, linux-nfs, linux-arch, linux-mm, alsa-devel WARN uses sometimes use KERN_<level> but mostly don't have any prefix. Change the WARN macros and the warn_slowpath function to preface KERN_WARNING and remove all the KERN_<level> uses from WARN sites. Neatening clean up of include/asm-generic/bug.h Update WARN macros Add KERN_WARNING to WARN output Remove any KERN_<level> from WARN uses Coalesce formats Align WARN arguments Add some missing newlines to WARN uses Add some missing first test argument (1, fmt, args) to WARN uses Joe Perches (39): include/asm-generic/bug.h: Update WARN macros arch/alpha: Update WARN uses arch/arm: Update WARN uses arch/powerpc: Update WARN uses arch/x86: Update WARN uses drivers/acpi: Update WARN uses drivers/base: Update WARN uses drivers/block: Update WARN uses drivers/cpuidle: Update WARN uses drivers/firewire: Update WARN uses drivers/firmware: Update WARN uses drivers/gpio: Update WARN uses drivers/gpu/drm: Update WARN uses drivers/media/video: Update WARN uses drivers/mfd: Update WARN uses drivers/net/can: Update WARN uses drivers/net/usb: Update WARN uses drivers/net/wireless/iwlwifi: Update WARN uses drivers/regulator: Update WARN uses drivers/scsi/fcoe: Update WARN uses drivers/staging: Update WARN uses drivers/usb/musb: Update WARN uses drivers/video/omap2/dss: Update WARN uses fs/nfsd: Update WARN uses fs/notify/inotify: Update WARN uses fs/sysfs: Update WARN uses fs/proc: Update WARN uses fs: Update WARN uses include/linux/device.h: Update WARN uses kernel/irq: Update WARN uses kernel/panic.c: Update warn_slowpath to use %pV kernel: Update WARN uses lib: Update WARN uses mm: Update WARN uses net/core/dev.c: Update WARN uses net/ipv4/tcp.c: Update WARN uses net/mac80211: Update WARN uses net/rfkill/input.c: Update WARN uses sound/soc/codecs/wm_hubs.c: Update WARN uses arch/alpha/kernel/pci-sysfs.c | 14 ++-- arch/arm/mach-davinci/clock.c | 4 +- arch/arm/mach-davinci/da830.c | 2 +- arch/arm/mach-davinci/da850.c | 12 ++-- arch/arm/mach-omap2/clkt_clksel.c | 12 ++-- arch/arm/mach-omap2/clock.c | 16 ++-- arch/arm/mach-omap2/devices.c | 4 +- arch/arm/mach-omap2/omap_hwmod.c | 47 +++++++----- arch/arm/mach-omap2/pm34xx.c | 7 +- arch/arm/mach-omap2/serial.c | 2 +- arch/arm/mach-omap2/timer-gp.c | 3 +- arch/arm/mach-tegra/clock.c | 3 +- arch/arm/mach-tegra/timer.c | 2 +- arch/arm/mach-u300/padmux.c | 14 +--- arch/arm/plat-omap/omap-pm-noop.c | 10 ++-- arch/powerpc/kernel/hw_breakpoint.c | 4 +- arch/x86/kernel/acpi/boot.c | 2 +- arch/x86/kernel/apic/apic.c | 2 +- arch/x86/kernel/apic/es7000_32.c | 2 +- arch/x86/kernel/cpu/perf_event.c | 4 +- arch/x86/kernel/pci-calgary_64.c | 4 +- arch/x86/kernel/tsc_sync.c | 4 +- arch/x86/kernel/xsave.c | 2 +- arch/x86/mm/ioremap.c | 5 +- arch/x86/mm/pageattr-test.c | 2 +- arch/x86/mm/pageattr.c | 5 +- arch/x86/platform/sfi/sfi.c | 4 +- drivers/acpi/ec.c | 4 +- drivers/base/class.c | 4 +- drivers/base/core.c | 5 +- drivers/base/memory.c | 4 +- drivers/base/sys.c | 10 +-- drivers/block/floppy.c | 2 +- drivers/cpuidle/driver.c | 2 +- drivers/firewire/core-transaction.c | 6 +- drivers/firmware/dmi_scan.c | 2 +- drivers/gpio/gpiolib.c | 4 +- drivers/gpio/it8761e_gpio.c | 2 +- drivers/gpu/drm/drm_crtc_helper.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 3 +- drivers/gpu/drm/radeon/evergreen.c | 2 +- drivers/gpu/drm/radeon/r100.c | 4 +- drivers/gpu/drm/radeon/r300.c | 2 +- drivers/gpu/drm/radeon/r600.c | 4 +- drivers/gpu/drm/radeon/radeon_fence.c | 3 +- drivers/gpu/drm/radeon/radeon_ttm.c | 3 +- drivers/gpu/drm/radeon/rs400.c | 2 +- drivers/gpu/drm/radeon/rs600.c | 4 +- drivers/media/video/s5p-fimc/fimc-core.c | 2 +- drivers/media/video/sr030pc30.c | 2 +- drivers/mfd/ezx-pcap.c | 5 +- drivers/net/can/mscan/mscan.c | 2 +- drivers/net/usb/ipheth.c | 2 +- drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 3 +- drivers/net/wireless/iwlwifi/iwl-agn-sta.c | 6 +- drivers/net/wireless/iwlwifi/iwl-tx.c | 6 +- drivers/net/wireless/iwlwifi/iwl3945-base.c | 2 +- drivers/regulator/core.c | 3 +- drivers/scsi/fcoe/libfcoe.c | 2 +- drivers/staging/memrar/memrar_allocator.c | 2 +- drivers/staging/solo6x10/solo6010-v4l2-enc.c | 2 +- drivers/usb/musb/musb_host.c | 6 +- drivers/video/omap2/dss/core.c | 3 +- fs/bio.c | 2 +- fs/buffer.c | 2 +- fs/nfsd/nfs4state.c | 3 +- fs/notify/inotify/inotify_fsnotify.c | 4 +- fs/proc/generic.c | 9 +-- fs/super.c | 5 +- fs/sysfs/dir.c | 3 +- fs/sysfs/file.c | 4 +- fs/sysfs/group.c | 4 +- fs/sysfs/symlink.c | 10 +-- include/asm-generic/bug.h | 101 ++++++++++++++++---------- include/linux/device.h | 2 +- kernel/irq/chip.c | 2 +- kernel/irq/manage.c | 2 +- kernel/notifier.c | 2 +- kernel/panic.c | 40 +++++------ kernel/pm_qos_params.c | 6 +- lib/debugobjects.c | 21 +++--- lib/iomap.c | 2 +- lib/kobject.c | 9 +-- lib/kobject_uevent.c | 4 +- lib/list_debug.c | 24 +++---- lib/plist.c | 12 ++-- mm/percpu.c | 4 +- mm/vmalloc.c | 5 +- net/core/dev.c | 3 +- net/ipv4/tcp.c | 16 ++--- net/mac80211/agg-tx.c | 5 +- net/mac80211/iface.c | 4 +- net/mac80211/mlme.c | 2 +- net/mac80211/rx.c | 4 +- net/mac80211/tx.c | 4 +- net/mac80211/util.c | 4 +- net/mac80211/work.c | 4 +- net/rfkill/input.c | 5 +- sound/soc/codecs/wm_hubs.c | 2 +- 99 files changed, 319 insertions(+), 330 deletions(-) -- 1.7.3.1.g432b3.dirty ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 02/39] arch/alpha: Update WARN uses 2010-10-30 21:08 [PATCH 00/39] Cleanup WARN #defines Joe Perches @ 2010-10-30 21:08 ` Joe Perches 2010-10-30 21:17 ` Matt Turner 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2010-10-30 21:08 UTC (permalink / raw) To: Jiri Kosina Cc: Richard Henderson, Ivan Kokshaysky, Matt Turner, linux-alpha, linux-kernel Coalesce long formats. Align arguments. Signed-off-by: Joe Perches <joe@perches.com> --- arch/alpha/kernel/pci-sysfs.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c index b899e95..6f30ac3 100644 --- a/arch/alpha/kernel/pci-sysfs.c +++ b/arch/alpha/kernel/pci-sysfs.c @@ -44,10 +44,9 @@ static int __pci_mmap_fits(struct pci_dev *pdev, int num, if (start < size && size - start >= nr) return 1; - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d " - "(size 0x%08lx)\n", - current->comm, sparse ? " sparse" : "", start, start + nr, - pci_name(pdev), num, size); + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d (size 0x%08lx)\n", + current->comm, sparse ? " sparse" : "", start, start + nr, + pci_name(pdev), num, size); return 0; } @@ -261,10 +260,9 @@ static int __legacy_mmap_fits(struct pci_controller *hose, if (start < size && size - start >= nr) return 1; - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d " - "(size 0x%08lx)\n", - current->comm, sparse ? " sparse" : "", start, start + nr, - hose->index, size); + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d (size 0x%08lx)\n", + current->comm, sparse ? " sparse" : "", start, start + nr, + hose->index, size); return 0; } -- 1.7.3.1.g432b3.dirty ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 02/39] arch/alpha: Update WARN uses 2010-10-30 21:08 ` [PATCH 02/39] arch/alpha: Update WARN uses Joe Perches @ 2010-10-30 21:17 ` Matt Turner 2010-10-30 21:28 ` Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Matt Turner @ 2010-10-30 21:17 UTC (permalink / raw) To: Joe Perches Cc: Jiri Kosina, Richard Henderson, Ivan Kokshaysky, linux-alpha, linux-kernel On Sat, Oct 30, 2010 at 5:08 PM, Joe Perches <joe@perches.com> wrote: > Coalesce long formats. > Align arguments. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > arch/alpha/kernel/pci-sysfs.c | 14 ++++++-------- > 1 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c > index b899e95..6f30ac3 100644 > --- a/arch/alpha/kernel/pci-sysfs.c > +++ b/arch/alpha/kernel/pci-sysfs.c > @@ -44,10 +44,9 @@ static int __pci_mmap_fits(struct pci_dev *pdev, int num, > > if (start < size && size - start >= nr) > return 1; > - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d " > - "(size 0x%08lx)\n", > - current->comm, sparse ? " sparse" : "", start, start + nr, > - pci_name(pdev), num, size); > + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d (size 0x%08lx)\n", > + current->comm, sparse ? " sparse" : "", start, start + nr, > + pci_name(pdev), num, size); > return 0; > } > > @@ -261,10 +260,9 @@ static int __legacy_mmap_fits(struct pci_controller *hose, > > if (start < size && size - start >= nr) > return 1; > - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d " > - "(size 0x%08lx)\n", > - current->comm, sparse ? " sparse" : "", start, start + nr, > - hose->index, size); > + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d (size 0x%08lx)\n", > + current->comm, sparse ? " sparse" : "", start, start + nr, > + hose->index, size); > return 0; > } > > -- > 1.7.3.1.g432b3.dirty What's accomplished here? It looks like you joined two strings (which were separated as to not overflow the 80-char limit) and spaced a couple lines over. I don't see how this makes anything clearer. Matt ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 02/39] arch/alpha: Update WARN uses 2010-10-30 21:17 ` Matt Turner @ 2010-10-30 21:28 ` Joe Perches 2010-10-31 1:11 ` Michael Cree 0 siblings, 1 reply; 6+ messages in thread From: Joe Perches @ 2010-10-30 21:28 UTC (permalink / raw) To: Matt Turner Cc: Jiri Kosina, Richard Henderson, Ivan Kokshaysky, linux-alpha, linux-kernel On Sat, 2010-10-30 at 17:17 -0400, Matt Turner wrote: > On Sat, Oct 30, 2010 at 5:08 PM, Joe Perches <joe@perches.com> wrote: > > Coalesce long formats. > > Align arguments. > > Signed-off-by: Joe Perches <joe@perches.com> > > arch/alpha/kernel/pci-sysfs.c | 14 ++++++-------- > > 1 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c > > @@ -44,10 +44,9 @@ static int __pci_mmap_fits(struct pci_dev *pdev, int num, > > - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d " > > - "(size 0x%08lx)\n", > > - current->comm, sparse ? " sparse" : "", start, start + nr, > > - pci_name(pdev), num, size); > > + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d (size 0x%08lx)\n", > > + current->comm, sparse ? " sparse" : "", start, start + nr, > > + pci_name(pdev), num, size); > > @@ -261,10 +260,9 @@ static int __legacy_mmap_fits(struct pci_controller *hose, > > - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d " > > - "(size 0x%08lx)\n", > > - current->comm, sparse ? " sparse" : "", start, start + nr, > > - hose->index, size); > > + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d (size 0x%08lx)\n", > > + current->comm, sparse ? " sparse" : "", start, start + nr, > > + hose->index, size); > What's accomplished here? It looks like you joined two strings (which > were separated as to not overflow the 80-char limit) and spaced a > couple lines over. > > I don't see how this makes anything clearer. Hi Matt. This one is just a whitespace cleanup. Ignore it or nack it if you want. The patch was generated mostly by script to standardize the WARN uses. Long formats are generally preferred for grep. In this case, there's no particular value in the format coalescing because the output isn't going to be found with a grep. There were other patches that are actual defect fixes. cheers, Joe ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 02/39] arch/alpha: Update WARN uses 2010-10-30 21:28 ` Joe Perches @ 2010-10-31 1:11 ` Michael Cree 2010-10-31 1:38 ` Joe Perches 0 siblings, 1 reply; 6+ messages in thread From: Michael Cree @ 2010-10-31 1:11 UTC (permalink / raw) To: Joe Perches Cc: Matt Turner, Jiri Kosina, Richard Henderson, Ivan Kokshaysky, linux-alpha, linux-kernel On 31/10/10 10:28, Joe Perches wrote: > On Sat, 2010-10-30 at 17:17 -0400, Matt Turner wrote: >> On Sat, Oct 30, 2010 at 5:08 PM, Joe Perches<joe@perches.com> wrote: >>> Coalesce long formats. >>> Align arguments. >>> Signed-off-by: Joe Perches<joe@perches.com> >>> arch/alpha/kernel/pci-sysfs.c | 14 ++++++-------- >>> 1 files changed, 6 insertions(+), 8 deletions(-) >>> diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c >>> @@ -44,10 +44,9 @@ static int __pci_mmap_fits(struct pci_dev *pdev, int num, >>> - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d " >>> - "(size 0x%08lx)\n", >>> - current->comm, sparse ? " sparse" : "", start, start + nr, >>> - pci_name(pdev), num, size); >>> + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on %s BAR %d (size 0x%08lx)\n", >>> + current->comm, sparse ? " sparse" : "", start, start + nr, >>> + pci_name(pdev), num, size); >>> @@ -261,10 +260,9 @@ static int __legacy_mmap_fits(struct pci_controller *hose, >>> - WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d " >>> - "(size 0x%08lx)\n", >>> - current->comm, sparse ? " sparse" : "", start, start + nr, >>> - hose->index, size); >>> + WARN(1, "process \"%s\" tried to map%s 0x%08lx-0x%08lx on hose %d (size 0x%08lx)\n", >>> + current->comm, sparse ? " sparse" : "", start, start + nr, >>> + hose->index, size); >> What's accomplished here? It looks like you joined two strings (which >> were separated as to not overflow the 80-char limit) and spaced a >> couple lines over. >> >> I don't see how this makes anything clearer. > > This one is just a whitespace cleanup. > Ignore it or nack it if you want. I prefer the original indenting as it clearly stands the `1' on its own thus emphasising that the warning will be printed everytime the WARN() statement is executed. I surmise that the original programmer thought it important to emphasise this as it might not be considered the "normal" usage of WARN(). I agree that, in general, coalesced strings aid grepping but this broken string isn't one that will benefit. I recall the story that when someone pointed out parallel fifths (an absolute no-no according to the rules of classical harmony) in a composition of Beethoven's to Beethoven himself, Beethoven was so indignant that he proceeded to compose a set of variations on a theme where in each variation he extensively violated one of the rules of classical harmony, and wrote down the bottom of each variation "You Ass" to the authority who had made the rule. The moral here is that when a _competent_ programmer breaks the "style guide" they are most likely doing so for a very good reason. Thus, those who would submit whitespace fix patches should take time to ask themselves why the original programmer has laid out the code in such a fashion and learn from it, before submitting so-called "fixes". Cheers Michael. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 02/39] arch/alpha: Update WARN uses 2010-10-31 1:11 ` Michael Cree @ 2010-10-31 1:38 ` Joe Perches 0 siblings, 0 replies; 6+ messages in thread From: Joe Perches @ 2010-10-31 1:38 UTC (permalink / raw) To: Michael Cree Cc: Matt Turner, Jiri Kosina, Richard Henderson, Ivan Kokshaysky, linux-alpha, linux-kernel On Sun, 2010-10-31 at 14:11 +1300, Michael Cree wrote: > On 31/10/10 10:28, Joe Perches wrote: > > On Sat, 2010-10-30 at 17:17 -0400, Matt Turner wrote: > >> On Sat, Oct 30, 2010 at 5:08 PM, Joe Perches<joe@perches.com> wrote: > >>> Coalesce long formats. > >>> Align arguments. > >> What's accomplished here? It looks like you joined two strings (which > >> were separated as to not overflow the 80-char limit) and spaced a > >> couple lines over. > > This one is just a whitespace cleanup. > > Ignore it or nack it if you want. > I prefer the original indenting as it clearly stands the `1' on its own > thus emphasising that the warning will be printed everytime the WARN() > statement is executed. I surmise that the original programmer thought > it important to emphasise this as it might not be considered the > "normal" usage of WARN(). Actually, WARN(1, foo) is by far the most common use of WARN. More than half of all uses. $ grep -rP --include=*.[ch] -oh "\bWARN\s*\(" * | wc -l 286 $ grep -rP --include=*.[ch] -oh "\bWARN\s*\(\w+" * | \ sort | uniq -c | sort -rn | head -5 173 WARN(1 8 WARN(IS_ERR 4 WARN(ret 4 WARN(priv 4 WARN(local > The moral here is that when a _competent_ programmer breaks the "style > guide" they are most likely doing so for a very good reason. Thus, > those who would submit whitespace fix patches should take time to ask > themselves why the original programmer has laid out the code in such a > fashion and learn from it, before submitting so-called "fixes". Code is decidedly not music. This stuff is mostly craft not art and consistency in craft has value. You might also note I didn't call it a fix, I called it a whitespace cleanup. And again, I don't much care if it's applied to arch/alpha or not. I haven't had an alpha since I gave away my AlphaStation a decade ago. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-31 1:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-30 21:08 [PATCH 00/39] Cleanup WARN #defines Joe Perches 2010-10-30 21:08 ` [PATCH 02/39] arch/alpha: Update WARN uses Joe Perches 2010-10-30 21:17 ` Matt Turner 2010-10-30 21:28 ` Joe Perches 2010-10-31 1:11 ` Michael Cree 2010-10-31 1:38 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).