Re: New style dpalloc/hostalloc routines (diff).

[Pantelis Antoniou <panto@intracom.gr>]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

Paul Mackerras wrote:

>Pantelis Antoniou writes:
>
>
>
>>+# Support new type of routines, usable from modules
>>+bool 'Use new type dpalloc routines()' CONFIG_NEW_DPALLOC
>>+bool 'Use new type hostalloc routines()' CONFIG_NEW_HOSTALLOC
>>+if [ "$CONFIG_NEW_DPALLOC" = "y" -o "$CONFIG_NEW_HOSTALLOC" = "y" ]; then
>>+  define_bool CONFIG_CPM_RHEAP y
>>+fi
>>
>>
>
>I don't want to see config options that select between different
>internal implementations of the same thing.  Either your new routines
>are better, and we'll use them, or they are worse, and we'll use the
>old ones.  Having a config option just leads to tons of ifdefs
>throughout the code, which makes it harder to read and understand.
>Having two implementations of the same thing is just bloat.
>
>Similarly, I don't like the way all your new routines have a "new_"
>prefix on the name.  You should be thinking of replacing the existing
>routines rather than providing an alternative implementation with a
>different name.  Where you have changed the API, either fix the
>drivers or provide a compatibility routine.
>
>The way it looks at the moment, it seems that you don't really have
>the conviction that your code is better than what is there already.
>Please redo your patch so that it just replaces the old routines.  And
>please don't send it as a bkpatch since they are impossible to read, a
>plain diff -u is much better.
>
>Paul.
>
>
>
>
>
>
OK

Here is the updated patch in diff format as per your suggestions.

Any more suggestions?

Pantelis




[-- Attachment #2: linuxppc_2_4_devel-dpalloc.patch.gz --]
[-- Type: application/x-gunzip, Size: 8109 bytes --]