This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: mallopt indentation bug?
- From: Carlos O'Donell <carlos_odonell at mentor dot com>
- To: Jeff Law <law at redhat dot com>, Alexandre Oliva <aoliva at redhat dot com>
- Cc: <libc-alpha at sources dot redhat dot com>
- Date: Wed, 5 Sep 2012 10:40:27 -0400
- Subject: Re: mallopt indentation bug?
- References: <orobll4bb9.fsf@livre.localdomain> <5046BA17.7070304@redhat.com>
On 9/4/2012 10:33 PM, Jeff Law wrote:
> On 09/04/2012 05:28 PM, Alexandre Oliva wrote:
>> The assignment to no_dyn_threshold is indented as if it was meant to be
>> run only in the else branch, but since C is not Python, it's run
>> unconditionally. That's not necessarily a bug, but since indentation is
>> misleading here, I figured I'd better fix it one way or the other, and I
>> decided to add braces to match the intent implied by indentation, rather
>> than reindent.
>>
>> Patch pushed to lxoliva/malloc-probes-bz742038. Ok for master?
>>
>> for ChangeLog
>> from Alexandre Oliva <aoliva@redhat.com>
>>
>> * malloc/malloc.c (__libc_mallopt) <M_MMAP_THRESHOLD>: Do not
>> change internal state upon failure.
>> ---
>>
>> malloc/malloc.c | 6 ++++--
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>>
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index 0f1796c..dd83551 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
Merge copyright years.
>> @@ -4766,8 +4766,10 @@ int __libc_mallopt(int param_number, int value)
>> if((unsigned long)value > HEAP_MAX_SIZE/2)
>> res = 0;
>> else
>> - mp_.mmap_threshold = value;
>> - mp_.no_dyn_threshold = 1;
>> + {
>> + mp_.mmap_threshold = value;
>> + mp_.no_dyn_threshold = 1;
>> + }
>> break;
Alexandre, Could you please also fix the indentation for M_MMAP_MAX?
> This looks correct to me.
Agreed. It's wrong to turn off the dynamic threshold if you don't set the threshold.
> If we don't change the mmap threshold there's no reason to turn of the dynamic mmap threshold heuristics.
>
> I doubt I count as a senior developer for glibc, so it'd be best to wait for someone else to chime in before pushing.
You are too modest.
Your seniority is not what is relevant.
What is relevant is the technical correctness of your review and the consensus that it is correct for the project.
Cheers,
Carlos.
--
Carlos O'Donell
Mentor Graphics / CodeSourcery
carlos_odonell@mentor.com
carlos@codesourcery.com
+1 (613) 963 1026