Re: [PATCH] Cleanup code duplication in malloc on fallback to use another arena

[Alexandre Oliva <aoliva at redhat dot com>]

On Aug 30, 2012, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:

> +static mstate
> +get_another_arena(mstate ar_ptr, size_t bytes)

I'd have preferred to place this in arena.c instead, where other arena
selection code is.  Would you consider doing that along with the
following fix?

> @@ -3038,23 +3046,10 @@ __libc_memalign(size_t alignment, size_t bytes)
> +    ar_ptr = get_another_arena (ar_ptr, bytes);
> +    if (__builtin_expect(ar_ptr != NULL, 1)) {
>        p = _int_memalign(ar_ptr, alignment, bytes);
> @@ -3088,23 +3083,10 @@ __libc_valloc(size_t bytes)
> +    ar_ptr = get_another_arena (ar_ptr, bytes);
> +    if (__builtin_expect(ar_ptr != NULL, 1)) {
>        p = _int_memalign(ar_ptr, pagesz, bytes);
> @@ -3136,23 +3118,10 @@ __libc_pvalloc(size_t bytes)
> +    ar_ptr = get_another_arena (ar_ptr, bytes + 2*pagesz + MINSIZE);
> +    if (__builtin_expect(ar_ptr != NULL, 1)) {
>        p = _int_memalign(ar_ptr, pagesz, rounded_bytes);

Of these 3 functions, that impose additional alignment requirements,
only the last one is passing to get_another_arena the same size passed
to arena_get.  I think this is a mistake (cut&pasto?)

How about computing and saving the size passed to arena_get in each of
these 3 functions, and passing the saved value to get_another_arena?

Now, how about renaming get_another_arena to say arena_get_retry, or
arena_get3 ;-) for greater similarity with the other functions that
choose an arena?


Thanks!

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer