Re: [PATCH 1/1] BEFS: Global conversion to pr_foo()

From: Joe Perches
Date: Tue Mar 11 2014 - 15:53:16 EST


On Tue, 2014-03-11 at 20:43 +0100, Fabian Frederick wrote:
> -All printk(KERN_foo converted to pr_foo().
> -Add pr_fmt and remove redundant prefixes.
[]
> diff --git a/fs/befs/debug.c b/fs/befs/debug.c

> @@ -31,7 +32,7 @@ befs_error(const struct super_block *sb, const char *fmt, ...)
> va_list args;
> char *err_buf = kmalloc(ERRBUFSIZE, GFP_KERNEL);
> if (err_buf == NULL) {
> - printk(KERN_ERR "could not allocate %d bytes\n", ERRBUFSIZE);
> + pr_err("could not allocate %d bytes\n", ERRBUFSIZE);

You could also get rid of these OOM messages as
there's a generic one for all .alloc failures.

> @@ -39,7 +40,7 @@ befs_error(const struct super_block *sb, const char *fmt, ...)
> vsnprintf(err_buf, ERRBUFSIZE, fmt, args);
> va_end(args);
>
> - printk(KERN_ERR "BeFS(%s): %s\n", sb->s_id, err_buf);
> + pr_err("(%s): %s\n", sb->s_id, err_buf);

Maybe use %pV too and get rid of the alloc/free
altogether like this uncompiled/untested patch below:

---

fs/befs/befs.h | 3 +++
fs/befs/debug.c | 54 +++++++++++++++++++-----------------------------------
2 files changed, 22 insertions(+), 35 deletions(-)

diff --git a/fs/befs/befs.h b/fs/befs/befs.h
index b266428..3a7813a 100644
--- a/fs/befs/befs.h
+++ b/fs/befs/befs.h
@@ -88,8 +88,11 @@ enum befs_err {

/****************************/
/* debug.c */
+__printf(2, 3)
void befs_error(const struct super_block *sb, const char *fmt, ...);
+__printf(2, 3)
void befs_warning(const struct super_block *sb, const char *fmt, ...);
+__printf(2, 3)
void befs_debug(const struct super_block *sb, const char *fmt, ...);

void befs_dump_super_block(const struct super_block *sb, befs_super_block *);
diff --git a/fs/befs/debug.c b/fs/befs/debug.c
index 622e737..a8f8d7f 100644
--- a/fs/befs/debug.c
+++ b/fs/befs/debug.c
@@ -23,43 +23,36 @@

#include "befs.h"

-#define ERRBUFSIZE 1024
-
void
befs_error(const struct super_block *sb, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;
- char *err_buf = kmalloc(ERRBUFSIZE, GFP_KERNEL);
- if (err_buf == NULL) {
- printk(KERN_ERR "could not allocate %d bytes\n", ERRBUFSIZE);
- return;
- }

va_start(args, fmt);
- vsnprintf(err_buf, ERRBUFSIZE, fmt, args);
- va_end(args);

- printk(KERN_ERR "BeFS(%s): %s\n", sb->s_id, err_buf);
- kfree(err_buf);
+ vaf.fmt = fmt;
+ vaf.args = &args;
+
+ pr_err("(%s): %pV\n", sb->s_id, &vaf);
+
+ va_end(args);
}

void
befs_warning(const struct super_block *sb, const char *fmt, ...)
{
+ struct va_format vaf;
va_list args;
- char *err_buf = kmalloc(ERRBUFSIZE, GFP_KERNEL);
- if (err_buf == NULL) {
- printk(KERN_ERR "could not allocate %d bytes\n", ERRBUFSIZE);
- return;
- }

va_start(args, fmt);
- vsnprintf(err_buf, ERRBUFSIZE, fmt, args);
- va_end(args);

- printk(KERN_WARNING "BeFS(%s): %s\n", sb->s_id, err_buf);
+ vaf.fmt = fmt;
+ vaf.args = &args;
+
+ pr_warn("(%s): %pV\n", sb->s_id, &vaf);

- kfree(err_buf);
+ va_end(args);
}

void
@@ -67,26 +60,17 @@ befs_debug(const struct super_block *sb, const char *fmt, ...)
{
#ifdef CONFIG_BEFS_DEBUG

+ struct va_format vaf;
va_list args;
- char *err_buf = NULL;
-
- if (BEFS_SB(sb)->mount_opts.debug) {
- err_buf = kmalloc(ERRBUFSIZE, GFP_KERNEL);
- if (err_buf == NULL) {
- printk(KERN_ERR "could not allocate %d bytes\n",
- ERRBUFSIZE);
- return;
- }

- va_start(args, fmt);
- vsnprintf(err_buf, ERRBUFSIZE, fmt, args);
- va_end(args);
+ va_start(args, fmt);

- printk(KERN_DEBUG "BeFS(%s): %s\n", sb->s_id, err_buf);
+ vaf.fmt = fmt;
+ vaf.args = &args;

- kfree(err_buf);
- }
+ pr_debug("(%s): %pV\n", sb->s_id, &vaf);

+ va_end(args);
#endif //CONFIG_BEFS_DEBUG
}



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/