Re: [PATCH] introduce sys_membarrier(): process-wide memorybarrier (v10)

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck@xxxxxxxxxxxxxxxxxx) wrote:
> On Mon, Apr 05, 2010 at 01:23:55PM -0700, Randy Dunlap wrote:
> > Paul E. McKenney wrote:
> > > On Mon, Apr 05, 2010 at 03:10:57PM -0400, Mathieu Desnoyers wrote:
> > >> * Randy Dunlap (randy.dunlap@xxxxxxxxxx) wrote:
> > >>> On Mon, 5 Apr 2010 13:57:37 -0400 Mathieu Desnoyers wrote:
> > > 
> > > [ . . . ]
> > > 
> > >>>> +#else /* #ifdef CONFIG_SMP */
> > >>> I don't know that we have a known convention for that, but I would use:
> > >>>
> > >>> #else /* not CONFIG_SMP */
> > >>>
> > >>> or
> > >>>
> > >>> #else /* !CONFIG_SMP */
> > >>>
> > >>>> +
> > >>>> +SYSCALL_DEFINE1(membarrier, unsigned int, flags)
> > >>>> +{
> > >>>> +	return 0;
> > >>>> +}
> > >>>> +
> > >>>> +#endif /* #else #ifdef CONFIG_SMP */
> > 
> > or just:
> > 
> > #endif /* #else #ifdef CONFIG_SMP : tell the reader that the #else part of the #ifdef CONFIG_SMP just ended */
> > 
> > ad nauseum.
> 
> You lost me on this one.
> 
> > >>> and:
> > >>>
> > >>> #endif /* CONFIG_SMP */
> > >>>
> > >>> The "#else #ifdef" is both ugly and too wordy IMO.
> > > 
> > > The extra words make it very clear that we are in at the end of the #else
> > > clause of a #ifdef with the given condition.  With "#endif /* CONFIG_SMP
> > > */", is the immediately preceding code compiled under CONFIG_SMP or
> > > !CONFIG_SMP?  You have to dig back and see whether or not there is a
> > > #else clause.
> > > 
> > > But there is no accounting for taste.  ;-)
> > 
> > IYHO.
> 
> Indeed, in both directions.

I strongly prefer Paul's approach, because it represents ifdef nesting much more
clearly, but I'll follow the coding style used by sched.c for these bits.

So maybe his approach is too verbose, but the

#ifdef X
#else /* !X */
#endif /* X */

is just a pain to follow with large chunks of code, especially when #ifndef
comes into play.

Thanks,

Mathieu

> 
> 							Thanx, Paul

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
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/