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

Blue Swirl blauwirbel at gmail.com
Tue Dec 21 21:58:54 CET 2010


On Tue, Dec 21, 2010 at 10:38 AM, Mark Cave-Ayland
<mark.cave-ayland at siriusit.co.uk> wrote:
> A casual glance at the source code suggests that pointers returned by ofmem_malloc() are aligned, but in fact they are not
> because sizeof(alloc_desc_t) is added to the final pointer return value. Create some additional padding after the previous value
> of ofmem->next_malloc so we can store the alloc_desc_t underneath the final return pointer in memory, thus ensuring that the
> final pointer is aligned correctly.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland at siriusit.co.uk>
> ---
>  openbios-devel/libopenbios/ofmem_common.c |   21 +++++++++++++--------
>  1 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/openbios-devel/libopenbios/ofmem_common.c b/openbios-devel/libopenbios/ofmem_common.c
> index 20a4cba..a4e199c 100644
> --- a/openbios-devel/libopenbios/ofmem_common.c
> +++ b/openbios-devel/libopenbios/ofmem_common.c
> @@ -33,6 +33,11 @@ static inline size_t ALIGN_SIZE(size_t x, size_t a)
>     return (x + a - 1) & ~(a-1);
>  }
>
> +static inline void * ALIGN_PTR(uintptr_t x, size_t a)

Uppercase name suggests a macro, please use lowercase.

> +{
> +    return (void *)((x + a - 1) & ~(a-1));

I'd add spaces inside a-1.

> +}
> +
>  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.

>                *pp = (**pp).next;
> -               return ret;
> +               return (void *)ret;
>        }
>
>        top = ofmem_arch_get_heap_top();
>
> -       if( pointer2cell(ofmem->next_malloc) + size > top ) {
> +       ret = (char *)ALIGN_PTR((uintptr_t)ofmem->next_malloc + sizeof(alloc_desc_t), CONFIG_OFMEM_MALLOC_ALIGN);
> +       if( pointer2cell((void *)ret) + size > top ) {
>                printk("out of malloc memory (%x)!\n", size );
>                return NULL;
>        }
>
> -       d = (alloc_desc_t*) ofmem->next_malloc;
> +       d = (alloc_desc_t*)((uintptr_t)ret - sizeof(alloc_desc_t));
>        ofmem->next_malloc += size;
>
>        d->next = NULL;
>        d->size = size;
>
> -       ret = (char*)d + sizeof(alloc_desc_t);
> -       memset( ret, 0, size - sizeof(alloc_desc_t) );
> +       memset( (char *)ret, 0, size - sizeof(alloc_desc_t) );
>
> -       return ret;
> +       return (void *)ret;
>  }
>
>  void ofmem_free( void *ptr )
> --
> 1.7.1
>
>
> --
> OpenBIOS                 http://openbios.org/
> Mailinglist:  http://lists.openbios.org/mailman/listinfo
> Free your System - May the Forth be with you
>



More information about the OpenBIOS mailing list