Re: [PATCH] setup_per_cpu_areas in 2.5.8pre3

Andries.Brouwer@cwi.nl
Wed, 17 Apr 2002 10:24:29 +0200 (MEST)


> Now that I am writing anyway, one of the changes I needed
> to compile 2.5.8pre3 is the following.

Yeah, better patch below

Good.

> Of course the real fix is to remove the #ifdef's,
> maybe using a weak symbol instead, or some other construction
> that defines an empty default that can be replaced by an actual
> routine.

Not unless you make it as readable as the current code. Having magic
appearing functions sounds cool, but beware that the cure might be
worse than the disease.

But maybe I do not think that the current code is very readable.
Probably because just before fixing this compilation problem I had
to fix a different one where atomic_dec_and_lock was undefined,
and one finds a forest of #ifdef's in spinlock.h.

#ifdef's are evil. You have one, and there are two possible sources.
Easy and readable. You have two, and there are four. Already a small
effort to check that indeed all four combinations are OK. That was
what went wrong in the setup_per_cpu_areas case. You have three and
it is almost certain that someone forgets to check all possible
eight cases.

#ifdef's hide source from the compiler, so that when stuff compiles
for the developer it need not compile for the next person who comes
along. We have a kernel compilation project because of that.

So, if we can replace a certain type of #ifdef use by a different
formal construction, where all source is seen by the compiler,
that might well be progress.

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