linux-kernel - Re: [PATCH 07/10] trace_syscalls: init print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4B21B172.2080500@cn.fujitsu.com>
Date:	Fri, 11 Dec 2009 10:41:54 +0800
From:	Lai Jiangshan <laijs@...fujitsu.com>
To:	rostedt@...dmis.org
CC:	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Masami Hiramatsu <mhiramat@...hat.com>,
	Jason Baron <jbaron@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 07/10] trace_syscalls: init print_fmt

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