Re: [updated patch] malloc per-thread cache ready for review

[Carlos O'Donell <carlos at redhat dot com>]

On 06/28/2017 09:35 PM, Siddhesh Poyarekar wrote:
> On Thursday 29 June 2017 06:46 AM, DJ Delorie wrote:
>> Siddhesh Poyarekar <siddhesh@gotplt.org> writes:
>>> haha, interesting, because I remember Florian and I had this discussion
>>> about the utility of SXID_IGNORE six months ago when I designed the
>>> scope control for the envvars.  Lets have Florian explain this.
>>
>> Changed to SXID_ERASE.
>>
>>> The braces should be on the next line:
>>
>> Fixed.
>>
>>>>>> +static void __attribute__ ((section ("__libc_thread_freeres_fn")))
>>>>>
>>>>> __attribute__ should be on its own line.
>>>>
>>>> Likewise, I was copying the formatting used elsewhere in malloc.  What
>>>> is the correct formatting for that line?  Do I need to fix the other
>>>> existing instances also?
>>>
>>> Sure, but in a separate (obvious) patch.  The malloc code was in a
>>> completely different format when it was ported in from dlmalloc and
>>> since then it has been a bit of a mishmash that needs a cleanup.
>>
>> I scanned all of glibc, and the same-line syntax seems to be nearly
>> ubiquitous, with only one example of it on a separate line.  I couldn't
>> find any mention of attributes in the GNU or GLIBC coding standards
>> either...
>>
>> I'm certainly willing to do a global tweak patch later, but... is this
>> documented somewhere?
> 
> I thought it should have been but now that you've forced me to think
> harder, my impression may be due to the way macro sections were written
> in some math functions.
> 
> Looks good to me, please commit.

I would like to do one more pass of review on this final patch just to
make sure we didn't miss anything.

-- 
Cheers,
Carlos.