public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@elte.hu>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC patch 3/5] ftrace trace event add missing semicolumn
Date: Wed, 5 Jan 2011 03:08:02 +0100	[thread overview]
Message-ID: <20110105020759.GG2911@nowhere> (raw)
In-Reply-To: <20110105001837.GA9737@Krystal>

On Tue, Jan 04, 2011 at 07:18:37PM -0500, Mathieu Desnoyers wrote:
> * Frederic Weisbecker (fweisbec@gmail.com) wrote:
> > On Tue, Jan 04, 2011 at 06:16:32PM -0500, Mathieu Desnoyers wrote:
> > > Add a missing semicolumn at the end of a ftrace definition.
> > > 
> > > We currently are not seeing any impact of this missing semicolumn because extra
> > > semicolumns appear all over the place in the code generated from TRACE_EVENT
> > > within ftrace stages.
> > > 
> > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > CC: Steven Rostedt <rostedt@goodmis.org>
> > > CC: Frederic Weisbecker <fweisbec@gmail.com>
> > > CC: Ingo Molnar <mingo@elte.hu>
> > > CC: Thomas Gleixner <tglx@linutronix.de>
> > > ---
> > >  include/trace/ftrace.h |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > Index: linux-2.6-lttng/include/trace/ftrace.h
> > > ===================================================================
> > > --- linux-2.6-lttng.orig/include/trace/ftrace.h
> > > +++ linux-2.6-lttng/include/trace/ftrace.h
> > > @@ -69,7 +69,7 @@
> > >  #undef DEFINE_EVENT
> > >  #define DEFINE_EVENT(template, name, proto, args)	\
> > >  	static struct ftrace_event_call	__used		\
> > > -	__attribute__((__aligned__(4))) event_##name
> > > +	__attribute__((__aligned__(4))) event_##name;
> > 
> > But DEFINE_EVENT() calls are supposed to be ";" terminated, no?
> 
> Currently yes, but if you look at the preprocessor output currently generated by
> the current TRACE_EVENT()/DEFINE_EVENT() scheme, there are useless ";" added all
> over the place. I have a patch later in my queue that proposes removal of these
> extra ";" as a cleanup of the TRACE_EVENT() semantic, but I'm keeping it for
> later because it removes the extra ";" at the end of each TRACE_EVENT()
> instance (and thus is more intrusive code-wise).
> 
> Adding this semicolumn here ensures that all Ftrace macros are consistent wrt
> semicolumns. We can get away without consistency currently exactly because the
> current scheme adds many useless semicolumns between each TRACE_EVENT().

Are you sure you want to put so much time on this?

This will require a massive change for the sole win of removing double ";"
in generated code. This won't optimize much the build, and it will make the things
not so much more readable for very rare people who dare to have interest into the
TRACE_EVENT generated code. That notwithstanding the obfuscation of that generated
code resides more in the lack of indentation and newlines than in double
semicolons that we barely notice.

Hm?

  reply	other threads:[~2011-01-05  2:08 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-04 23:16 [RFC patch 0/5] Trace event fixes and cleanups Mathieu Desnoyers
2011-01-04 23:16 ` [RFC patch 1/5] trace event block fix unassigned field Mathieu Desnoyers
2011-01-05 15:09   ` Jeff Moyer
2011-01-05 19:34     ` Mathieu Desnoyers
2011-01-05 19:57       ` Jeff Moyer
2011-01-04 23:16 ` [RFC patch 2/5] trace event skb " Mathieu Desnoyers
2011-01-04 23:16 ` [RFC patch 3/5] ftrace trace event add missing semicolumn Mathieu Desnoyers
2011-01-05  0:00   ` Frederic Weisbecker
2011-01-05  0:18     ` Mathieu Desnoyers
2011-01-05  2:08       ` Frederic Weisbecker [this message]
2011-01-05  2:35         ` Mathieu Desnoyers
2011-01-05  2:58           ` Frederic Weisbecker
2011-01-05 13:52             ` Mathieu Desnoyers
2011-01-05 15:02               ` Frederic Weisbecker
2011-01-05 19:56                 ` Mathieu Desnoyers
2011-01-05 23:40                   ` Frederic Weisbecker
2011-01-05 23:57                     ` Steven Rostedt
2011-01-06 18:08                     ` Mathieu Desnoyers
2011-01-05  3:01         ` Valdis.Kletnieks
2011-01-05  3:10           ` Frederic Weisbecker
2011-01-05  6:37             ` Valdis.Kletnieks
2011-01-05 13:56               ` Mathieu Desnoyers
2011-01-04 23:16 ` [RFC patch 4/5] tracepoint trace event add missing comma Mathieu Desnoyers
2011-01-04 23:16 ` [RFC patch 5/5] trace event sched: remove TP_perf_assign Mathieu Desnoyers
2011-01-05  9:58   ` Peter Zijlstra
2011-01-05 13:28     ` Mathieu Desnoyers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110105020759.GG2911@nowhere \
    --to=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox