Re: [PATCH 1/2 v4] scsi: add __print_hex() to ftrace

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 2010-04-01 at 20:40 +0900, Kei Tokunaga wrote:
> This is the same patch from v1.
> 
> __print_hex() prints values in an array in hex (w/o '0x') (space separated)
> EX) 92 33 32 f3 ee 4d
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@jp.fujitsu.com>
> Signed-off-by: Kei Tokunaga <tokunaga.keiich@jp.fujitsu.com>
> 
> ---
> 
>  linux-2.6.34-rc3-trace-kei/include/linux/ftrace_event.h |    3 +++
>  linux-2.6.34-rc3-trace-kei/include/trace/ftrace.h       |    3 +++
>  linux-2.6.34-rc3-trace-kei/kernel/trace/trace_output.c  |   15 +++++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff -puN include/linux/ftrace_event.h~add__print_hex_to_ftrace include/linux/ftrace_event.h
> --- linux-2.6.34-rc3-trace/include/linux/ftrace_event.h~add__print_hex_to_ftrace	2010-03-31 10:03:14.000000000 +0900
> +++ linux-2.6.34-rc3-trace-kei/include/linux/ftrace_event.h	2010-03-31 10:03:14.000000000 +0900
> @@ -25,6 +25,9 @@ const char *ftrace_print_flags_seq(struc
>  const char *ftrace_print_symbols_seq(struct trace_seq *p, unsigned long val,
>  				     const struct trace_print_flags *symbol_array);
>  
> +const char *ftrace_print_hex_seq(struct trace_seq *p,
> +				 const unsigned char *buf, int len);
> +
>  /*
>   * The trace entry - the most basic unit of tracing. This is what
>   * is printed in the end as a single line in the trace output, such as:
> diff -puN include/trace/ftrace.h~add__print_hex_to_ftrace include/trace/ftrace.h
> --- linux-2.6.34-rc3-trace/include/trace/ftrace.h~add__print_hex_to_ftrace	2010-03-31 10:03:14.000000000 +0900
> +++ linux-2.6.34-rc3-trace-kei/include/trace/ftrace.h	2010-03-31 10:03:14.000000000 +0900
> @@ -198,6 +198,9 @@
>  		ftrace_print_symbols_seq(p, value, symbols);		\
>  	})
>  
> +#undef __print_hex
> +#define __print_hex(buf, buf_len) ftrace_print_hex_seq(p, buf, buf_len)
> +
>  #undef DECLARE_EVENT_CLASS
>  #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
>  static notrace enum print_line_t					\
> diff -puN kernel/trace/trace_output.c~add__print_hex_to_ftrace kernel/trace/trace_output.c
> --- linux-2.6.34-rc3-trace/kernel/trace/trace_output.c~add__print_hex_to_ftrace	2010-03-31 10:03:14.000000000 +0900
> +++ linux-2.6.34-rc3-trace-kei/kernel/trace/trace_output.c	2010-04-01 20:38:30.000000000 +0900
> @@ -355,6 +355,21 @@ ftrace_print_symbols_seq(struct trace_se
>  }
>  EXPORT_SYMBOL(ftrace_print_symbols_seq);
>  
> +const char *
> +ftrace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len)
> +{
> +	int i;
> +	const char *ret = p->buffer + p->len;
> +
> +	for (i = 0; i < buf_len; i++)
> +		trace_seq_printf(p, "%s%2.2x", i == 0 ? "" : " ", buf[i]);
> +
> +	trace_seq_putc(p, 0);

I would not put the putc(p, 0) into a helper function. This terminates
the buffer, and strings added to the trace_seq will go after the '\0'.

What we need is this in trace_seq.h:

static inline trace_seq_terminate(struct trace_seq *s)
{
	if (!s->full)
		s->buffer[s->len] = 0;
}

This will add a '0' to the buffer after the last string, but will not
increment length while doing it. Thus, new data added to the buffer
would still be appended correctly.

-- Steve

> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(ftrace_print_hex_seq);
> +
>  #ifdef CONFIG_KRETPROBES
>  static inline const char *kretprobed(const char *name)
>  {
> 
> _
> 
> 
>