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

[Pantelis Antoniou <panto@intracom.gr>]

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.
>
>
>
>
>
Well that's easy to do since that's the way it is in my tree.

The only reason for the new_ prefixes and the config options was to
make it easy for people to test them.

I didn't sent it only as a bkpatch, a following mail had the patch in
diff format.

Anyway I'll repost them as per you suggestions...

Pantelis


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/