This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] i386: Increase MALLOC_ALIGNMENT to 16 [BZ #21120]


On 06/30/2017 09:35 AM, H.J. Lu wrote:
> On Fri, Jun 30, 2017 at 6:05 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 06/30/2017 07:54 AM, H.J. Lu wrote:
>>> On Thu, Jun 29, 2017 at 1:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> Are you suggesting 3 i386 header files to define MALLOC_ALIGNMENT?
>>>>
>>>> 1. generic i386.
>>>> 2. nptl i386.
>>>> 3. hurd i386.
>>>>
>>>> That is very weird.
>>
>> I hope my explanation below makes the difference clearer.
>>
>>> I built and checked them for i686 with GCC 7.  The biggest challenge is to
>>> support GCC 7, not the header file itself.
>>
>> We have an existing malloc-machine.h for this purpose.
>>
>> The design question is:
>>
>> (a) Should we add a new file malloc-alignment.h to specify a new parameter.
>>     - Adds new file malloc-alignment.h.
>>     - 2 new files.
> 
> And define MALLOC_ALIGNMENT for i386 in one place.
> 
>>
>> (b) Should we use existing malloc-machine.h to include the new parameter?
>>     - Extends existing malloc-machine.h to new sysdep dirs.
>>     - 3 new files.
> 
> And define  MALLOC_ALIGNMENT for i386 in 3 difference places.
> If we need to support a new OS or threading library, we need to
> define MALLOC_ALIGNMENT for i386 in another place.
> 
>> You are arguing for (a) and I argue for (b).
>>
>> Use 1 less file but require a new header.
>> Use 1 more file but use existing headers.
>>
>> I argue that it's simpler and easier for developers if we avoid additional headers
>> and use existing headers.
>>
> 
> Because your suggestion defines MALLOC_ALIGNMENT for i386 in
> more than one place, I argue it is more complex and more error
> prone than a single MALLOC_ALIGNMENT definition for i386.  With
> my approach, it is fixed once for all.

Can't they all equally just include 'sysdeps/i386/malloc-machine.h'?

/* Include the base i386 definitions.  */
#include <sysdeps/i386/malloc-machine.h>

So you have one definition in one place?

In sysdeps/nptl/malloc-machine.h we already have:
#include <sysdeps/generic/malloc-machine.h>

So we already use a layered approach to avoid multiple definitions.

-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]