[PATCH 2/2] Fix padding initialization in nano_malloc?

Joey Ye joey.ye.cc@gmail.com
Thu Jan 12 02:24:00 GMT 2017


Joe,

Thanks for this enhancement to the comments, which is clearer than the original.

Only a minor issue to me: restrictly c->size should be the next word
after the padding, instead of the last word of the padding. After
correcting this problem I am OK with the patch.

Thanks,
Joey

On Wed, Jan 11, 2017 at 2:57 PM, Joe Seymour <joe.s@somniumtech.com> wrote:
> Hi Joey,
>
> On 10/01/2017 03:45, Joey Ye wrote:
>> Thanks for reading the code and starting this discussion. But IMHO you
>> misunderstood the data structure thus this patch is not valid.
>
> Thanks for the review! I agree and withdraw that patch.
>
> I was thinking of the padding as being like a variable-length field in the
> structure, which led me to assume that the offset was being read/written from
> the beginning of the padding, instead of the end.
>
> To try and stop anyone else making the same mistake I've prepared the following
> patch, which adds some more comments to nano-mallocr.c. Note that I haven't
> verified that the existing code ensures that the size of padding is at least
> CHUNK_OFFSET.
>
> What do you think?
>
> Thanks,
>
> Joe Seymour
>
> ---
>  newlib/libc/stdlib/nano-mallocr.c |   23 ++++++++++++++++++++---
>  1 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/newlib/libc/stdlib/nano-mallocr.c b/newlib/libc/stdlib/nano-mallocr.c
> index 457eb88..4497eb6 100644
> --- a/newlib/libc/stdlib/nano-mallocr.c
> +++ b/newlib/libc/stdlib/nano-mallocr.c
> @@ -128,7 +128,7 @@ typedef struct malloc_chunk
>       *          ------------------
>       *          | Padding for    |
>       *          | alignment      |
> -     *          | holding neg    |
> +     *          | ending with neg|
>       *          | offset to size |
>       *          ------------------
>       * mem_ptr->| point to next  |
> @@ -187,9 +187,23 @@ extern void * nano_pvalloc(RARG size_t s);
>
>  static inline chunk * get_chunk_from_ptr(void * ptr)
>  {
> +    /* Assume that there is no explicit padding in the
> +       chunk, and that the chunk starts at ptr - CHUNK_OFFSET.  */
>      chunk * c = (chunk *)((char *)ptr - CHUNK_OFFSET);
> -    /* Skip the padding area */
> -    if (c->size < 0) c = (chunk *)((char *)c + c->size);
> +
> +    if (c->size < 0) {
> +      /* c->size being negative indicates that there is explicit padding in
> +         the chunk; c->size is actually the last CHUNK_OFFSET bytes in that
> +         padding.
> +
> +         To find the true start of the chunk we need to subtract the size of
> +         any remaining padding, and then CHUNK_OFFSET from c.  This is
> +         equivalent to subtracting the total size of the padding from c,
> +         because the size of any remaining padding is the total size of the
> +         padding minus CHUNK_OFFSET.  The end of the padding should have been
> +         initialized with this negative offset.  */
> +      c = (chunk *)((char *)c + c->size);
> +    }
>      return c;
>  }
>
> @@ -314,6 +328,9 @@ void * nano_malloc(RARG malloc_size_t s)
>
>      if (offset)
>      {
> +        /* Initialize the end of the padding with a negative offset to the
> +           start of the chunk.  The rest of the padding is not initialized.
> +           Note that the size of the padding must be at least CHUNK_OFFSET.  */
>          *(long *)((char *)r + offset) = -offset;
>      }
>
> --
> 1.7.1



More information about the Newlib mailing list