Re: [PATCH 1/7] printk: clean up return value

From: Joe Perches
Date: Wed Oct 07 2009 - 12:38:13 EST

Next message: Rik van Riel: "Re: IO scheduler based IO controller V10"
Previous message: Alan Stern: "Re: 2.6.31.2 : error when ending apcupsd"
In reply to: Alan Jenkins: "Re: [PATCH 1/7] printk: clean up return value"
Next in thread: Alan Jenkins: "Re: [PATCH 1/7] printk: clean up return value"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, 2009-10-07 at 08:36 -0700, Linus Torvalds wrote:
> On Wed, 7 Oct 2009, Alan Jenkins wrote:
> > We could fix this up, but it seems pointless. Callers don't really care
> > about the extra characters added by printk(). Instead, let's return the
> > length of the original message as formatted (and possibly truncated)
> > by vscnprintf().
> Or we could just change it to 'void'. As Joe Perches says, nobody really
> cares deeply enough for this to generally even matter.

Here are changes to make printk/vprintk return void
to the only uses I found:

arch/arm/mach-iop13xx/pci.c | 2 +-
arch/arm/mach-iop13xx/setup.c | 2 +-
drivers/char/mem.c | 6 ++----
drivers/md/md.c | 3 +--
drivers/md/raid5.c | 3 ++-
drivers/net/e100.c | 9 +++++----
include/linux/kernel.h | 16 ++++++++--------
include/net/sctp/sctp.h | 6 +++---
kernel/lockdep.c | 4 ++--
kernel/printk.c | 10 +++-------
net/sunrpc/svc.c | 9 +++------
11 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/arch/arm/mach-iop13xx/pci.c b/arch/arm/mach-iop13xx/pci.c
index 4873f26..eb58d0a 100644
--- a/arch/arm/mach-iop13xx/pci.c
+++ b/arch/arm/mach-iop13xx/pci.c
@@ -28,7 +28,7 @@
#include <mach/pci.h>

#define IOP13XX_PCI_DEBUG 0
-#define PRINTK(x...) ((void)(IOP13XX_PCI_DEBUG && printk(x)))
+#define PRINTK(x...) do { if (IOP13XX_PCI_DEBUG) printk(x); } while (0)

u32 iop13xx_atux_pmmr_offset; /* This offset can change based on strapping */
u32 iop13xx_atue_pmmr_offset; /* This offset can change based on strapping */
diff --git a/arch/arm/mach-iop13xx/setup.c b/arch/arm/mach-iop13xx/setup.c
index 5c147fb..f6595ad 100644
--- a/arch/arm/mach-iop13xx/setup.c
+++ b/arch/arm/mach-iop13xx/setup.c
@@ -29,7 +29,7 @@

#define IOP13XX_UART_XTAL 33334000
#define IOP13XX_SETUP_DEBUG 0
-#define PRINTK(x...) ((void)(IOP13XX_SETUP_DEBUG && printk(x)))
+#define PRINTK(x...) do { if (IOP13XX_SETUP_DEBUG) printk(x); } while (0)

/* Standard IO mapping for all IOP13XX based systems
*/
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index a074fce..6fd98d0 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -851,10 +851,8 @@ static ssize_t kmsg_write(struct file * file, const char __user * buf,
ret = -EFAULT;
if (!copy_from_user(tmp, buf, count)) {
tmp[count] = 0;
- ret = printk("%s", tmp);
- if (ret > count)
- /* printk can add a prefix */
- ret = count;
+ printk("%s", tmp);
+ ret = count;
}
kfree(tmp);
return ret;
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 26ba42a..02173fd 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -51,8 +51,7 @@
#include "bitmap.h"

#define DEBUG 0
-#define dprintk(x...) ((void)(DEBUG && printk(x)))
-
+#define dprintk(x...) do { if (DEBUG) printk(x); } while (0)

#ifndef MODULE
static void autostart_arrays(int part);
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 9482980..1e4b018 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -94,7 +94,8 @@
#define __inline__
#endif

-#define printk_rl(args...) ((void) (printk_ratelimit() && printk(args)))
+#define printk_rl(args...) \
+ do { if (printk_ratelimit()) printk(args); } while (0)

/*
* We maintain a biased count of active stripes in the bottom 16 bits of
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index 679965c..9e35650 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -198,10 +198,11 @@ module_param(use_io, int, 0);
MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
MODULE_PARM_DESC(eeprom_bad_csum_allow, "Allow bad eeprom checksums");
MODULE_PARM_DESC(use_io, "Force use of i/o access mode");
-#define DPRINTK(nlevel, klevel, fmt, args...) \
- (void)((NETIF_MSG_##nlevel & nic->msg_enable) && \
- printk(KERN_##klevel PFX "%s: %s: " fmt, nic->netdev->name, \
- __func__ , ## args))
+#define DPRINTK(nlevel, klevel, fmt, args...) do { \
+ if (NETIF_MSG_##nlevel & nic->msg_enable) \
+ printk(KERN_##klevel PFX "%s: %s: " fmt, \
+ nic->netdev->name, __func__ , ## args); \
+ } while (0)

#define INTEL_8255X_ETHERNET_DEVICE(device_id, ich) {\
PCI_VENDOR_ID_INTEL, device_id, PCI_ANY_ID, PCI_ANY_ID, \
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index d3cd23f..c75bad4 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -236,9 +236,9 @@ extern struct pid *session_of_pgrp(struct pid *pgrp);
#define FW_INFO "[Firmware Info]: "

#ifdef CONFIG_PRINTK
-asmlinkage int vprintk(const char *fmt, va_list args)
+asmlinkage void vprintk(const char *fmt, va_list args)
__attribute__ ((format (printf, 1, 0)));
-asmlinkage int printk(const char * fmt, ...)
+asmlinkage void printk(const char * fmt, ...)
__attribute__ ((format (printf, 1, 2))) __cold;

extern struct ratelimit_state printk_ratelimit_state;
@@ -262,12 +262,12 @@ extern int printk_delay_msec;

void log_buf_kexec_setup(void);
#else
-static inline int vprintk(const char *s, va_list args)
+static inline void vprintk(const char *s, va_list args)
__attribute__ ((format (printf, 1, 0)));
-static inline int vprintk(const char *s, va_list args) { return 0; }
-static inline int printk(const char *s, ...)
+static inline void vprintk(const char *s, va_list args) {}
+static inline void printk(const char *s, ...)
__attribute__ ((format (printf, 1, 2)));
-static inline int __cold printk(const char *s, ...) { return 0; }
+static inline void __cold printk(const char *s, ...) {}
static inline int printk_ratelimit(void) { return 0; }
static inline bool printk_timed_ratelimit(unsigned long *caller_jiffies, \
unsigned int interval_msec) \
@@ -389,7 +389,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#else
#define pr_devel(fmt, ...) \
- ({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
+ ({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); ; })
#endif

/* If you are writing a driver, please use dev_dbg instead */
@@ -403,7 +403,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
} while (0)
#else
#define pr_debug(fmt, ...) \
- ({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); 0; })
+ ({ if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); ; })
#endif

/*
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 8a6d529..9330780 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -276,9 +276,9 @@ struct sctp_mib {
#if SCTP_DEBUG
extern int sctp_debug_flag;
#define SCTP_DEBUG_PRINTK(whatever...) \
- ((void) (sctp_debug_flag && printk(KERN_DEBUG whatever)))
+ do { if (sctp_debug_flag) printk(KERN_DEBUG whatever); } while (0)
#define SCTP_DEBUG_PRINTK_IPADDR(lead, trail, leadparm, saddr, otherparms...) \
- if (sctp_debug_flag) { \
+ do { if (sctp_debug_flag) { \
if (saddr->sa.sa_family == AF_INET6) { \
printk(KERN_DEBUG \
lead "%pI6" trail, \
@@ -292,7 +292,7 @@ extern int sctp_debug_flag;
&saddr->v4.sin_addr.s_addr, \
otherparms); \
} \
- }
+ } while (0)
#define SCTP_ENABLE_DEBUG { sctp_debug_flag = 1; }
#define SCTP_DISABLE_DEBUG { sctp_debug_flag = 0; }

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 3815ac1..2f77f23 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -1281,8 +1281,8 @@ static void print_lock_class_header(struct lock_class *class, int depth)
if (class->usage_mask & (1 << bit)) {
int len = depth;

- len += printk("%*s %s", depth, "", usage_str[bit]);
- len += printk(" at:\n");
+ printk("%*s %s at:\n", depth, "", usage_str[bit]);
+ len += depth + strlen(usage_str[bit]) + 8;
print_stack_trace(class->usage_traces + bit, len);
}
}
diff --git a/kernel/printk.c b/kernel/printk.c
index f38b07f..276f40f 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -586,16 +586,13 @@ static int have_callable_console(void)
* See the vsnprintf() documentation for format string extensions over C99.
*/

-asmlinkage int printk(const char *fmt, ...)
+asmlinkage void printk(const char *fmt, ...)
{
va_list args;
- int r;

va_start(args, fmt);
- r = vprintk(fmt, args);
+ vprintk(fmt, args);
va_end(args);
-
- return r;
}

/* cpu currently holding logbuf_lock */
@@ -667,7 +664,7 @@ static inline void printk_delay(void)
}
}

-asmlinkage int vprintk(const char *fmt, va_list args)
+asmlinkage void vprintk(const char *fmt, va_list args)
{
int printed_len = 0;
int current_log_level = default_message_loglevel;
@@ -796,7 +793,6 @@ out_restore_irqs:
raw_local_irq_restore(flags);

preempt_enable();
- return printed_len;
}
EXPORT_SYMBOL(printk);
EXPORT_SYMBOL(vprintk);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 952f206..33cf786 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -953,25 +953,22 @@ static void svc_unregister(const struct svc_serv *serv)
/*
* Printk the given error with the address of the client that caused it.
*/
-static int
+static void
__attribute__ ((format (printf, 2, 3)))
svc_printk(struct svc_rqst *rqstp, const char *fmt, ...)
{
va_list args;
- int r;
char buf[RPC_MAX_ADDRBUFLEN];

if (!net_ratelimit())
- return 0;
+ return;

printk(KERN_WARNING "svc: %s: ",
svc_print_addr(rqstp, buf, sizeof(buf)));

va_start(args, fmt);
- r = vprintk(fmt, args);
+ vprintk(fmt, args);
va_end(args);
-
- return r;
}

/*

--
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/

Next message: Rik van Riel: "Re: IO scheduler based IO controller V10"
Previous message: Alan Stern: "Re: 2.6.31.2 : error when ending apcupsd"
In reply to: Alan Jenkins: "Re: [PATCH 1/7] printk: clean up return value"
Next in thread: Alan Jenkins: "Re: [PATCH 1/7] printk: clean up return value"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]