[OpenBIOS] [PATCH 1/5] Fix alignment of memory allocated by OFMEM's ofmem_malloc().

[Mark Cave-Ayland]

Blue Swirl wrote:

>> +static inline void * ALIGN_PTR(uintptr_t x, size_t a)
> 
> Uppercase name suggests a macro, please use lowercase.

Yeah... this is a bit tricky as the style in OFMEM is different from 
elsewhere. My thinking was to try and match the existing style and then 
possibly attempt a tidy-up later after things are bedded in just to keep 
the commits cleaner.

>> +{
>> +    return (void *)((x + a - 1) & ~(a-1));
> 
> I'd add spaces inside a-1.

Okay - will do.

>> +}
>> +
>>  static ucell get_ram_size( void )
>>  {
>>        ofmem_t *ofmem = ofmem_arch_get_private();
>> @@ -102,29 +107,29 @@ void* ofmem_malloc( size_t size )
>>
>>        /* waste at most 4K by taking an entry from the freelist */
>>        if( *pp && (**pp).size < size + 0x1000 ) {
>> -               ret = (char*)*pp + sizeof(alloc_desc_t);
>> -               memset( ret, 0, (**pp).size - sizeof(alloc_desc_t) );
>> +               ret = (char *)((uintptr_t)*pp + sizeof(alloc_desc_t));
>> +               memset( (char *)ret, 0, (**pp).size - sizeof(alloc_desc_t) );
> 
> There are a lot of char* and void* casts, perhaps type of ret should
> be void * to avoid some of them. At least (char *)ret shouldn't be
> necessary anyway.

I did think about that, but wasn't sure if this would cause funny 
pointer maths bugs. If you think that would be okay, I can give it a go.


ATB,

Mark.

-- 
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063

Sirius Labs: http://www.siriusit.co.uk/labs