[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