Re: [PATCH 07/10] trace_syscalls: init print_fmt

[Lai Jiangshan <laijs@cn.fujitsu.com>]

Steven Rostedt wrote:
> On Wed, 2009-12-09 at 15:15 +0800, Lai Jiangshan wrote:
>> Init print_fmt for trace_syscalls.
>> It will be used for replacing ->show_format().
> 
> This needs more explanation too.
> 
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>> ---
>> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
>> index 75289f3..dcd8699 100644
>> --- a/kernel/trace/trace_syscalls.c
>> +++ b/kernel/trace/trace_syscalls.c
>> @@ -191,6 +191,61 @@ int syscall_enter_format(struct ftrace_event_call *call, struct trace_seq *s)
>>  	return trace_seq_putc(s, '\n');
>>  }
>>  
>> +static
>> +int  __set_enter_print_fmt(struct syscall_metadata *entry, char *buf, int len)
>> +{
>> +	int i;
>> +	int pos = 0;
>> +
>> +	pos += snprintf(buf + pos, len > pos ? len - pos : 0, "\"");
>> +	for (i = 0; i < entry->nb_args; i++) {
>> +		pos += snprintf(buf + pos, len > pos ? len - pos : 0,
>> +				"%s: 0x%%0%zulx%s", entry->args[i],
>> +				sizeof(unsigned long),
>> +				i == entry->nb_args - 1 ? "" : ", ");
>> +	}
>> +	pos += snprintf(buf + pos, len > pos ? len - pos : 0, "\"");
>> +
>> +	for (i = 0; i < entry->nb_args; i++) {
>> +		pos += snprintf(buf + pos, len > pos ? len - pos : 0,
>> +				", ((unsigned long)(REC->%s))", entry->args[i]);
> 
> Yuck! 4 snprintf(buf + pos, len > pos ? len - pos: 0 ...
> 
> Please make a wrapper for that.

Do you mind if I use a macro?
Readability are still the same... 

static
int  __set_enter_print_fmt(struct syscall_metadata *entry, char *buf, int len)
{
	int i;
	int pos = 0;

#define BUF_PRINTF(fmt...)						\
do {									\
	pos += snprintf(buf + pos, len > pos ? len - pos : 0, fmt);	\
} while (0)

	BUF_PRINTF("\"");
	for (i = 0; i < entry->nb_args; i++) {
		BUF_PRINTF("%s: 0x%%0%zulx%s", entry->args[i],
				sizeof(unsigned long),
				i == entry->nb_args - 1 ? "\"" : ", ");
	}

	for (i = 0; i < entry->nb_args; i++)
		BUF_PRINTF(", ((unsigned long)(REC->%s))", entry->args[i]);

#under BUF_PRINTF

	/* return the length of print_fmt */
	return pos;
}

> 
> Actually, this could use the new trace_seq code if I separate the
> buffer. You would just need to do:
> 
> 	struct trace_seq s;

I think trace_seq can not help.
__set_enter_print_fmt() is called twice.
We calculate the length at the first time(don't write any thing),
then allocate memory and then really "snprintf" string into the buffer.

-- Lai

> 
> 	trace_seq_init(&s, buf, len);
> 
> 	trace_seq_putc(&s, '"');
> 	for (i = 0; i < entry->nb_args; i++)
> 		trace_seq_printf(&s, "%s: 0x%%0%zulx%s", entry->args[i],
> 				sizeof(unsigned long),
> 				i == entry->nb_args - 1 ? "" : ", ");
> 
> 	trace_seq_putc(&s, '"');
> 
> 	for (i = 0; i < entry->nb_args; i++)
> 		trace_seq_printf(&s, ", ((unsigned long)(REC->%s)",
> 				entry->args[i]);
> 
> 	return s.len;
> 
> 
> Looks much better, and less error prone.
> 
> -- Steve
>