Re: [PATCH 1/2] ftrace: nmi safe code modification

[Andrew Morton]

On Thu, 30 Oct 2008 16:58:55 -0400 (EDT)
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Thu, 30 Oct 2008, Andrew Morton wrote:
> > 
> > > +#ifndef __ASSEMBLY__
> > > +#define ftrace_nmi_enter()	do { } while (0)
> > > +#define ftrace_nmi_exit()	do { } while (0)
> > > +#endif
> > > ...
> > > +#ifndef __ASSEMBLY__
> > > +#define ftrace_nmi_enter()	do { } while (0)
> > > +#define ftrace_nmi_exit()	do { } while (0)
> > > +#endif
> > 
> > These could all be written in C.  If there's a reson to write them in
> > cpp then the `#ifndef __ASSEMBLY__' isn't really needed.
> 
> I could do the C macro, and you are right, I did not need the __ASSEMBLY__ 
> part. I guess that was me just being over-protective :-/
> 
> Which would you prefer?  Changing to C or removing the __ASSEMBLY__?

>From a general perspective, C is better.  Has typechecking, adds a ref
to the arguments which can prevent unused-var warnings, easier to read
and maintain, more likely to be commented, known about by debug info,
doesn't all get clumped into a single line in debug info, easier/safer
to uninline, blah, blah.

Also it seems a bit weird to do

#ifdef SOMETHING
#define foo(...) ...
#else
extern void foo(...);
#endif

Doing it in C has the downside that more things need to be visible at
the definition site, so more includes might be needed.  Often fixable
by uninlining.


I dunno.  People seem to instinctively reach for a macro without
thinking, because that's how grandpa did it or something.

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