Re: [PATCH v2] powerpc: change rheap functions to use ulongs instead of pointers

[Sylvain Munaut <tnt@246tNt.com>]

Kumar Gala wrote:
>
> On Apr 10, 2007, at 2:09 PM, Timur Tabi wrote:
>
>> Kumar Gala wrote:
>>
>>> What's the specific problem you are fixing?  Its not obvious that
>>> this patch is addressing a bug.
>>
>> problem != bug
>>
>> The problem is that a pointer implies something you can dereference. 
>> But the return value from rh_alloc() is only a pointer in a specific
>> circumstance which is not actually used in any current code.  So
>> *all* of the callers of rh_alloc() cast the return value to an
>> integer type anyway.
>>
>> In other words, it's wrong to use a pointer.  The value is a generic
>> number, and so the type needs to match that.  The code is just better
>> using ulongs.  Also, two redundant macros (IS_MURAM_ERR, etc) have
>> been removed and replaced with their generic counterpart (IS_ERR_VALUE).
>
> I consider this all code cleanup at this point since the code is
> functional at this point.  I don't disagree with any of your points,
> but this is cleanup.  We should take this all the way if we are going
> to do it.
I like this patch, I think the conversion to ulong does make sense (a
whole lot more than void * ...).

And I don't really agree that we need to "take it all the way" at once
... Even if later it's interesting to make it more global, the work that
is done here won't be lost so I'm in favor of merging that now.
Moving it to /lib is most likely going to take quite some time and that
should not prevent local cleaning of arch/powerpc

Removing all those cast make the core more readable imho. And writing
new code that use rheap is a whole lot more clear without having to
bother casting to/from void* each time you use a rh_ function ...


    Sylvain