Re: [patch 1/1] fs/proc: clean up printks

[Andrew Morton <akpm@linux-foundation.org>]

On Sun, 03 Feb 2013 12:59:45 -0800
Joe Perches <joe@perches.com> wrote:

> On Thu, 2013-01-31 at 14:20 -0800, akpm@linux-foundation.org wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Subject: fs/proc: clean up printks
> > 
> > - use pr_foo() throughout
> > 
> > - remove a couple of duplicated KERN_WARNINGs, via WARN(KERN_WARNING "...")
> > 
> > - nuke a few warnings which I've never seen happen, ever.
> 
> Ignorable trivial comments only:
> 
> o Use pr_warn instead of pr_warning

Yeah, I did that in the -fix patch.

> o Consider using "%s: ...", __func__, ...
>   instead of "function_name: ...", ...

Too lazy ;)

> o As printk.h is included by kernel.h, and that
>   is unlikely to be changed, it's probably not
>   useful/necessary to add #include <linux/printk.h>
>   anywhere

Too anal.

Failing to explicitly include the stuff you need often causes breakage
when header files are cleaned up, but we're unlikelly to weed printk.h
out of kernel.h.

> o Consider using #define pr_fmt(fmt) where useful

hm, where and how?

> o Consider coalescing formats

out of scope (ie: too lazy)

> o Consider realigning arguments after name changes

Fixed a couple.

--- a/fs/proc/generic.c~fs-proc-clean-up-printks-fix-fix
+++ a/fs/proc/generic.c
@@ -829,8 +829,8 @@ void remove_proc_entry(const char *name,
 		parent->nlink--;
 	de->nlink = 0;
 	WARN(de->subdir, "%s: removing non-empty directory "
-			"'%s/%s', leaking at least '%s'\n", __func__,
-			de->parent->name, de->name, de->subdir->name);
+			 "'%s/%s', leaking at least '%s'\n", __func__,
+			 de->parent->name, de->name, de->subdir->name);
 	pde_put(de);
 }
 EXPORT_SYMBOL(remove_proc_entry);
--- a/fs/proc/proc_sysctl.c~fs-proc-clean-up-printks-fix-fix
+++ a/fs/proc/proc_sysctl.c
@@ -997,7 +997,7 @@ static int sysctl_err(const char *path, 
 	vaf.va = &args;
 
 	pr_err("sysctl table check failed: %s/%s %pV\n",
-			path, table->procname, &vaf);
+	       path, table->procname, &vaf);
 
 	va_end(args);
 	return -EINVAL;
_