Re: [patch] clear direction flag for signal handlers

[Aggelos Economopoulos <aoiko@xxxxxxxxxxxxxx>]

On Wednesday 12 March 2008, Matthew Dillon wrote:
> :only differs in some code block and that block can be cleanly turned into a
> :function, do just that. Or, for example in sendsig() define a macro
> :is_vm86(regs) (or whatever), only define it as 0 in the appropriate vkernel
> :include and you're done. The compiler will just throw away the dead code, no
> :#ifdefs involved. Etc.
> 
>     Heh.  No way.  A macro used for conditional compilation is really
>     no different from an #ifdef.  We're definitely not going to do
>     that.

But it is different.

compare

if (has_vm(regs)) {
	code;
}

with

#ifndef _KERNEL_VIRTUAL
if (regs->tf_eflags & PSL_VM) {
	code;
}
#endif

In the first case you have to parse one conditional, the same conditional that
was there before vkernel was added to the tree. The macro just serves as an
optimal __predict_false(regs->tf_eflags & PSL_VM) that effectively does
__assume_false() and allows the compiler to remove the code at compile time.
There is no extra logic.

The second case adds an extra conditional that is evaluated at a different time
and uses different syntax than the rest of the code. Repeat a couple of times
(with #else too), intersperse some C conditionals and you end up with the kind
of mess that rightly gave #ifdef its bad name.

> :As for changes in one arch (assuming you mean platform here) breaking
> :another, well, although it's a real possibility I don't think it is
> :very probable. If you're making big changes, you'll normally end up updating
> :the vkernel side as well, so having a mostly-common codebase forces you to
> :think about the vkernel too from the start. Sorry, but I can't give a more
> :concrete answer to this hypothetic scenario.
> 
>     It's not only probable, it's virtually guaranteed to occur.  Trying to
>     share code through even moderate conditionalization is a big mistake
>     from the point of view of then having to maintain that code.  Sure,
>     some things you do want to try to share... if we had 50 different
>     platforms all sharing the same bit of code then it would make sense
>     to put that code in a machine independant directory.  But the last 
>     thing we want to do is have excessive cross-platform code sharing.

I suppose your argument here, like mine, is based on anecdotal evidence. So
I guess a reasonable answer would be "No, no, no, you're wrong, I'm right" ;)

> :Unfortunately this isn't very convincing, so I'll just claim that the
> :benefits from any code sharing outweigh any possibilities for accidental
> :breakage. When you fix a bug (or clean it up, or even add a feature) in
> :duplicated code you must remember to propagate this change to all duplicates.
> :I think most people will agree that relying on the programmer keep track of
> :duplicates is a recipe for lost bugfixes and diverging code copies.
> 
>     It's no better the other way.  When you make a change in a piece of
>     common code, you have no guarantee that your change will not break
>     platforms other then the one you tested on.  This is particularly true
>     of any bit of code with #ifdef's (or conditional compilation macros).

It seems to me that's just as true when you change code that is shared by
not just two, but by all platforms (i.e. generic code). I don't think
things break that often[*].

Since I don't think I can convince you to change your mind, I'll be polite
and not waste your time (and mine) by arguing the remaining points :)

Aggelos

[*] Unless you *force* code to be shared where it shouldn't but that's not
what I suggested.