Re: [Minios-devel] [UNIKRAFT PATCH v2] lib/ukalloc: fix wrong return value in uk_malloc_ifpages

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Costin,

good catch, thanks. Luckily, that doesn't lead to problems right now, 
because sizeof(uintptr_t) == sizeof(uinptr_t *) on x86_64, but it's 
bound to break eventually. I'll prepare a fix.
Cheers,
Florian

On 09/06/2018 09:35 AM, Costin Lupu wrote:
> Hi Florian,
>
> Just a minor thing, please see inline.
>
> On 08/30/2018 05:08 PM, Florian Schmidt wrote:
> > uk_malloc_ifpages returned the wrong pointer, because it added
> > sizeof(size_t) to a pointer of type size_t. Hence, the return value
> > wasn't offset from intptr by size_t bytes, but by size_t*size_t bytes.
> >
> > This patch makes intptr a uintptr_t in places in which its property as
> > size_t* isn't used.
> >
> > Signed-off-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>
> > ---
> >   lib/ukalloc/alloc.c | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/ukalloc/alloc.c b/lib/ukalloc/alloc.c
> > index 52e9a77..fb38bcb 100644
> > --- a/lib/ukalloc/alloc.c
> > +++ b/lib/ukalloc/alloc.c
> > @@ -167,7 +167,7 @@ static inline size_t uk_alloc_size_to_order(size_t size)
> >   
> >   void *uk_malloc_ifpages(struct uk_alloc *a, size_t size)
> >   {
> > -       size_t *intptr;
> > +       uintptr_t intptr;
> >         size_t order;
> >         size_t realsize = sizeof(order) + size;
> >   
> > @@ -176,13 +176,13 @@ void *uk_malloc_ifpages(struct uk_alloc *a, size_t size)
> >                 return NULL;
> >   
> >   	order = uk_alloc_size_to_order(realsize);
> > -       intptr = uk_palloc(a, order);
> > +       intptr = (uintptr_t)uk_palloc(a, order);
> >   
> >   	if (!intptr)
> >                 return NULL;
> >   
> > -	*intptr = order;
> > -       return intptr + sizeof(order);
> > +       *(size_t *)intptr = order;
> > +       return (void *)(intptr + sizeof(order));
> >   }
> >   
> >   void uk_free_ifpages(struct uk_alloc *a, void *ptr)
> > @@ -229,7 +229,7 @@ void *uk_realloc_ifpages(struct uk_alloc *a, void *ptr, 
> > size_t size)
> >   int uk_posix_memalign_ifpages(struct uk_alloc *a,
> >                                 void **memptr, size_t align, size_t size)
> >   {
> > -       size_t *intptr;
> > +       uintptr_t *intptr;
> I guess intptr should be of type uintptr_t (and not uintptr_t *).
>
> >         size_t realsize;
> >         size_t order;
> >   
> > @@ -259,7 +259,7 @@ int uk_posix_memalign_ifpages(struct uk_alloc *a,
> >         if (!intptr)
> >                 return ENOMEM;
> >   
> > -	*intptr = order;
> > +       *(size_t *)intptr = order;
> >         *memptr = (void *) ALIGN_UP((uintptr_t)intptr + sizeof(order), align);
> And here you would get rid of the cast to uintptr_t.
>
> >         return 0;
> >   }
>
> Cheers,
> Costin
--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel