linux-kernel - Re: trace/events: DECLARE vs DEFINE semantic

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 02 Dec 2009 14:19:38 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc:	akpm@...ux-foundation.org, Ingo Molnar <mingo@...e.hu>,
	mingo@...hat.com, hpa@...or.com, linux-kernel@...r.kernel.org,
	randy.dunlap@...cle.com, wcohen@...hat.com, fweisbec@...il.com,
	tglx@...utronix.de, jbaron@...hat.com, mhiramat@...hat.com,
	linux-tip-commits@...r.kernel.org, Christoph Hellwig <hch@....de>
Subject: Re: trace/events: DECLARE vs DEFINE semantic

On Wed, 2009-12-02 at 14:01 -0500, Mathieu Desnoyers wrote:
> * Steven Rostedt (rostedt@...dmis.org) wrote:
> > On Wed, 2009-12-02 at 13:06 -0500, Mathieu Desnoyers wrote:
> > > *
> > > Hrm. I wonder if having DEFINE_EVENT_CLASS is really worth having,
> > > considering that it really just does 2 things at once and may be
> > > confusing.
> > 
> > We keep it because that's what TRACE_EVENT currently is. It would suck
> > to have to replace every TRACE_EVENT there is now with a
> > DECLARE_EVENT_CLASS and DEFINE_EVENT. Although this would push
> > developers into using classes.
> 
> I agree that keeping something for backward compatibility is good, but
> what I dislike the most is the similarity between the
> DECLARE_EVENT_CLASS and DEFINE_EVENT_CLASS which have completely
> unrelated semantics. This is really misleading.

Not really, they are almost identical. But one creates an event with the
class, whereas the other does not. I find this quite convenient.

> > 
> > Egad No! It would make it a living nightmare. The internals reuse the
> > define macro, and there's no intermediate. By changing the
> > DECLARE_EVENT_CLASS to another name (SKETCH_EVENT_CLASS) we would have
> > to add something like this:
> > 
> > #define SKETCH_EVENT_CLASS(name, proto, args, tstruct, print) \
> > 	DECLARE_EVENT_CLASS(name, PARAMS(proto), PARAMS(args),\
> > 		PARAMS(tstruct), PARAMS(print))
> > 
> > We don't have a intermediate or "low level" macro in use here. Whatever
> > we give to the user is what we use.
> > 
> 
> Maybe we should consider having one. e.g.:
> 
> #ifdef CREATE_TRACE_POINTS
> 
> SKETCH_EVENT_CLASS maps to DEFINE_EVENT_CLASS
> 
> #else
> 
> SKETCH_EVENT_CLASS maps to DECLARE_EVENT_CLASS
> 
> #endif

And what? Make another level of needless abstraction? That's sure to not
confuse people.

> 
> > 
> > I think the kernel developers are smart enough to figure out that these
> > macros are not a typical DECLARE/DEFINE that is elsewhere. But I think
> > using the DECLARE/DEFINE names will give them a better idea of what is
> > happening than to make up something completely new.
> 
> In my opinion, re-using a well-known keyword (e.g. DECLARE/DEFINE) but
> applying a different semantic to what is generally agreed upon is a
> recipe for confusing developers and users, who will skip the review of
> some pieces of code assuming they already know what "DECLARE" and
> "DEFINE" stands for.
> 
> I argue here that the content of trace/events/ headers are _not_ per se
> declarations nor definitions, and hence they should not confuse people
> by using inappropriately well-known keywords. They are actually more
> evolved macros that can be turned in either a declaration or definition,
> depending if CREATE_TRACE_POINTS is declared.

And I argue that the semantics here are not too far off to what those
are. Yes, these macros behave differently if CREATE_TRACE_POINTS is
declared or not, but I argue that the average (and below average) kernel
developer is smart enough to understand this difference.


> 
> When I created the markers/tracepoints, Andrew Morton explained to me
> the importance of distinguishing DECLARE vs DEFINE macros. I would
> really like to hear his point of view on the current question.

I would like to hear Andrew's comments too, as well as anyone else.
Randy Dunlap seemed to already approve of these naming conventions, and
he's a pretty picky person too.

Randy, do you agree that the use of DECLARE/DEFINE here is fine, or do
you think that we should come up with a better naming. I do not want to
add any needless abstraction layer for the sake of naming. These macros
are confusing enough without that.

Or do you (or anyone else) have a better name?


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