Re: [patch] clear direction flag for signal handlers

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

: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.

: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.

: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).

:platform/{vkernel,pc32}/i386/procfs_machdep.c
:are identical

    This is almost machine-independant code already, it could probably
    be moved to the cpu-specific directory (/usr/src/sys/cpu/i386).


:platform/{vkernel,pc32}/i386/tls.c
:differ by one stub function and 1 include line

   This could probably be moved too.


:platform/{pc32/isa,vkernel/platform}/ipl_funcs.c
:differ in 3 include lines and one extra comment

    This is getting a bit more iffy.  The IPL functions are liable to
    change in the future in ways that will diverge.


:platform/{pc32/i386,vkernel/platform}/busdma_machdep.c
:the diffstat for those is
: busdma_machdep.c |   75 +++++++++++--------------------------------------------
: 1 file changed, 15 insertions(+), 60 deletions(-)
:and i386/busdma_machdep.c is 941 lines long
:The differences are an early panic for the vkernel version in
:free_bounce_page() an extra function and a block that is only needed in the
:pc32 case. Seems mergeable, but >80% of the code can be shared right now.
:
:platform/{pc32,vkernel}/i386/db_trace.c
:are similar, but not too promising.
:
:platform/{pc32,vkernel}/i386/genassym.c
:IMHO beg to have their common lines split out.
     
    None of these are good candidates, not even genassym.c.  As the code
    evolves changes made to these files are almost guaraneteed to diverge.
	 

:platform/{pc32/i386/machdep,vkernel/i386/cpu_regs}.c
:Lot's of functions that are pc32-only, vm86 differences seem easy to merge, a
:symbol difference (Maxmem vs physmem), sys_sigreturn() needs some thought.
:
:platform/{pc32,vkernel}/i386/vm_machdep.c
:extra casts in vkernel(?), some missing code, kvm_access_check() mergeable.
:
:platform/{pc32,vkernel}/i386/autoconf.c
:can share some lines
:
:platform/{pc32,vkernel}/i386/db_interface.c
:Debugger()
:
:platform/{pc32/isa,vkernel/i386}/npx.c
:would need some work.
:
:platform/{pc32,vkernel}/i386/trap.c
:a few functions are identical, trap()/user_trap() which is ~1/3 of the
:file is not easy to merge.

    None of these are good candidates, for the same reason.  The NPX code
    is a good example of why it isn't a good idea.  When I originally broke
    the vkernel off I spent several hours trying to share the npx code.
    It was a dismal failure.  Despite the fact that nearly every change
    to npx we make to pc32 has to be reflected in vkernel, I still far
    prefer them to be separate.


:pmap.c is significantly different (not surprisingly)
:
:Aggelos

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>