[OpenBIOS] [RFC 2/5] Fix for pointer2cell() and cell2pointer()

[Blue Swirl]

On Wed, Oct 27, 2010 at 8:56 PM, Andreas Färber <andreas.faerber at web.de> wrote:
> Am 27.10.2010 um 21:48 schrieb Blue Swirl:
>
>> On Tue, Oct 26, 2010 at 11:08 PM, Andreas Färber <andreas.faerber at web.de>
>> wrote:
>>>
>>> ---
>>>  include/kernel/stack.h |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/kernel/stack.h b/include/kernel/stack.h
>>> index 809ffe9..44fef0f 100644
>>> --- a/include/kernel/stack.h
>>> +++ b/include/kernel/stack.h
>>> @@ -31,8 +31,8 @@ typedef ucell phandle_t;
>>>
>>>
>>>  #ifdef NATIVE_BITWIDTH_EQUALS_HOST_BITWIDTH
>>> -#define pointer2cell(x) ((ucell)(x))
>>> -#define cell2pointer(x) ((u8 *)(x))
>>> +#define pointer2cell(x) ((ucell)(uintptr_t)(x))
>>> +#define cell2pointer(x) ((u8 *)(uintptr_t)(x))
>>
>> Perhaps this should be changed to inline function which returns void
>> *, then we may avoid a few casts.
>
> I wouldn't mind inline functions either.
>
> I doubt this quickfix is right at all though. It makes things compile with
> minimal impact, but these decisions seem seriously flawed. switch-arch and
> these macros all assume that just pointer size matters. IMO we would need
> CELL_BITWIDTH_EQUALS_POINTER_BITWIDTH instead and should be using
> CELL_BITWIDTH_SMALLER_THAN_POINTER_BITWIDTH (two #ifdefs below) for ppc64
> instead of making the equality code path (hardcoded in Makefile.target)
> handle the smaller-than case as well now.

Sounds about right.

> If we want to go ahead with my above patch I should at least add a TODO that
> this is wrong, no?

I think it's your decision if you want to create even better patches,
but I don't think this is too wrong, or at least we are not going to
wrong direction.