linux-kernel - Re: [PATCH -tip] tracing: use defined fields to print formats

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <alpine.DEB.2.00.0907202116350.17909@gandalf.stny.rr.com>
Date:	Mon, 20 Jul 2009 21:21:20 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Lai Jiangshan <laijs@...fujitsu.com>
cc:	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -tip] tracing: use defined fields to print formats


On Tue, 21 Jul 2009, Lai Jiangshan wrote:
> 
> There're 4 differences:
> 
> 1) the type of "common_type" is changed.
> It's brought by a bug `__common_field(int, type, 1);`
> It'll be fixed.
> 
> 2) Type Item[Len] ==> Type[Len] Item.
> (There are the same, )But it can't not be recovered as before.
> Because it's brought by `trace_define_field(event_call, #type "[" #len "]", #item,`
> I'll fix trace-cmd and make it supports these two style together.

I'll wait for your next patch and see if I can take a crack at getting it
back to the old way.

> 
> 3) double quotation marks for the format is missing.
> It'll be fixed.
> 
> 4) a line in events/ftrace/print/format is missing.
> It'll be fixed.
> 
> At the end(fixed patch), the only change is "Type Item[Len] ==> Type[Len] Item".
> 
> The values of this patch are:
> 1) A big function ftrace_format_##call() is defined for every event type.
> it waste a lot of memory. this patch saves these memory.

I agree.

> 
> 2) reduce coupling: ftrace_format_##call() and
> ftrace_define_fields_##call() are almost the same.
> We need to maintain them together, it's not good design.

I agree.

> 
> The difference between ftrace_format_##call() and
> ftrace_define_fields_##call() implies a bug.
> 
> Example: `__common_field(int, type, 1);` in
> ftrace_define_fields_##call() is a bug.

The problem is that this will not make it until 32, and 31 already has the 
type item[x] format. Which means tools will be expecting it that way. We 
can make the tools handle both, but I would really like to prevent that.

I totally agree that your patch is a nice clean up, and saves space and 
prevents more bugs. On the other hand, we are changing an API to users 
where all tools that parse this must be able to cope.

I'll wait for your next patch and see if I can't pull something out of my 
CPP magic hat that can save the day ;-)

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