|
From: Daniel J S. <dan...@ie...> - 2006-04-09 06:03:50
|
Ethan A Merritt wrote:
> On Saturday 08 April 2006 07:30 pm, Daniel Sebald wrote:
>
>
>>Anyway, I know that this is bloated.
>>My philosophy to these patches with conditional code has always been
>>that if one turns off the experimental feature it goes back *exactly*
>>to the code before the patch was applied.
>
>
> Right idea, but there is a better way to accomplish this.
>
> Step 1-5:
Got it. I can create a couple patches to reduce this the conditionals.
>
> - changing manifest constants into enums or #defines
>
This one I would definitely like to see in the case of gplt_x11.c. I.e., combine all the letter codes into one group of defines.
> I'm still learning this game myself, inspired by watching the development
> process of the linux kernel. Have a look yourself. The amazing thing is
> that the linux kernel has virtually no conditional segments despite a huge
> number of configuration options; their effects are hidden in definitions,
> header files, and in the choice of alternative source files to build from.
I have looked at the Linux code, having had to compile special kernels from time to time. It's some of the best code out there. (And their method of tracking bug fixes and notifying the public is excellent.)
>
>>> Find_Plot_In_Linked_List_By_Number(int plot_number)
>>> {
>>>+#ifdef FOO
>>> if (plot_number < 0) return NULL;
>>>+#endif
>
>
> The linux kernel approach would be to change the above line unconditionally
> to something like
> if (plot_number < MIN_PLOT_NUMBER) return NULL;
> And then in some configuration header file you would have
> #ifdef FOO
> #define MIN_PLOT_NUMBER 0
> #else
> #define MIN_PLOT_NUMBER (-MAXINT)
> #endif
Well, that is sort of the C header file model of things. Given how far the optimization of GPL compilers have come along, the case where MIN_PLOT_NUMBER is -MAXINT will be optimized out of the assembled code. I'd be fine with that approach to things.
> Yes. But as I said, I think that if the code is properly cleaned up
> there will only be a handful of places where such conditional code is needed.
A pre-patch is no problem. Let me work on it.
Dan
|