Re: [patch] clean hex output of ftrace

[Harvey Harrison]

On Wed, 2008-10-08 at 17:16 -0700, Joe Perches wrote:
> On Wed, 8 Oct 2008, Harvey Harrison wrote:
> > From: Harvey Harrison <harvey.harrison@xxxxxxxxx>
> > Subject: [PATCH] ftrace: Fix inversion of hex output and use common routines
> 
> You're quick...
> 
> >  	BUG_ON(len >= HEX_CHARS);
> 
> I think the BUG_ON is senseless.
> 

Agreed, it probably meant to say BUG_ON(len * 2 > HEX_CHARS -1)

But as this is only called from within a helper macro, it could be
changed to a build-time check instead of a runtime:

From: Harvey Harrison <harvey.harrison@xxxxxxxxx>
Subject: [PATCH] trace: add build-time check to avoid overrunning hex buffer

Remove the runtime BUG_ON and change to a compile-time check in
the macro that calls the hex format routine

[Noticed by Joe Perches]
Signed-off-by: Harvey Harrison <harvey.harrison@xxxxxxxxx>
---
 kernel/trace/trace.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 763f763..1ffbc24 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -395,7 +395,8 @@ trace_seq_putmem(struct trace_seq *s, void *mem, size_t len)
 	return len;
 }
 
-#define HEX_CHARS 17
+#define MAX_MEMHEX_BYTES	(8)
+#define HEX_CHARS		((MAX_MEMHEX_BYTES * 2) + 1)
 
 static int
 trace_seq_putmem_hex(struct trace_seq *s, void *mem, size_t len)
@@ -404,8 +405,6 @@ trace_seq_putmem_hex(struct trace_seq *s, void *mem, size_t len)
 	unsigned char *data = mem;
 	int i, j;
 
-	BUG_ON(len >= HEX_CHARS);
-
 #ifdef __BIG_ENDIAN
 	for (i = 0, j = 0; i < len; i++) {
 #else
@@ -1706,6 +1705,7 @@ do {							\
 
 #define SEQ_PUT_HEX_FIELD_RET(s, x)			\
 do {							\
+	BUILD_BUG_ON(sizeof(x) > MAX_MEMHEX_BYTES);	\
 	if (!trace_seq_putmem_hex(s, &(x), sizeof(x)))	\
 		return 0;				\
 } while (0)
-- 
1.6.0.2.471.g47a76



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