[OpenAFS-devel] Splitting out old Linux code

[Chas Williams (CONTRACTOR)]

In message <DA039332-7ABD-4C46-9441-7CC9B2960132@inf.ed.ac.uk>,Simon Wilkinson writes:
>I'm in the process of making some fairly deep changes to the Linux  
>cache manager, in the search of more performance, and I'm worried  
>about the implications for the 2.4 kernel series. Rather than  
>scattering the code with yet more #ifdefs, I was wondering how people  
>would feel about the following proposal.

i am against it but there really isnt anything else to do at this point.
in the future, people should try to follow the linux module and keep
the ifdef maze in a header file somewhere.  for instance, if you need
the current time and two kernels do it differently, dont write:

	osi_gettime()
	{
	#ifdef kernel_of_the_day
		style_one()
	#else some_other_kernel
		style_two()
	#else
		oh_god_help_me()
	#endif
	}

instead write:

	osi_gettime()
	{
		whatever_the_right_way_is_today()
	}

and in a header file make the necessary compat static inline (this might
be a gcc-ism, but the linux kernel only builds with gcc):

	#ifdef kernel_of_the_day
	static inline whatever_the_right_way_is_today()
	{
		older_kernel_method()l
	}
	#endif


for instance, for osi_Time(), in machdep.h we have:

	#define afs_hz HZ
	#include "h/sched.h"
	#if defined(HAVE_CURRENT_KERNEL_TIME)
	static inline time_t osi_Time(void) {
	    struct timespec xtime;
	    xtime = current_kernel_time();
	    return xtime.tv_sec;
	}
	#else
	#define osi_Time() (xtime.tv_sec)
	#endif

this is awful.  instead, in osi_somewhere.c write:

	time_t osi_Time(void) {
	    struct timespec xtime;

	    xtime = current_kernel_time();
	    return xtime.tv_sec;
	}

in machdep.h write:

	#define afs_hz HZ
	#include "h/sched.h"
	#if !defined(HAVE_CURRENT_KERNEL_TIME)

	struct timespec { int tv_sec, int tv_usec } xtime;	/* if necessary */

	static time_t long current_kernel_time(void) {
	    struct timespec __xtime; 
            __xtime.tv_sec = xtime.tv_sec;
            __xtime.tv_usec = 0;
	    return __xtime;
	}
	#endif


yes, this look ugly too, but the ugliness is confined to maintaining
the backward compatability and not in the parts of the code you would
normally read.  yes, there are some cases (like functions which change
calling interfaces) that you cant handle this way.  but i would like to
think this would go a long way to making things more readable.