Re: tunables signed/unsigned bug & patch

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

On 01/20/2017 04:46 PM, DJ Delorie wrote:
> 
> "Carlos O'Donell" <carlos@redhat.com> writes:
>> For some reason I thought mallopt would return the old value
>> via the return, but it clearly doesn't. My mistake.
> 
> I wouldn't be opposed to a mallopt_get() API but I think it's a bad idea
> in light of the tunables (should mallopt itself use the tunables
> infrastructure?).  A future tunables API to fetch the value from the
> bowels of glibc would be a much better solution, and allow for a full
> regression test of the tunables code anyway.
> 
>> So this gives you a relatively robust test that will very likely catch
>> the problem we saw.
> 
> Hmmm... I tested my patch by hacking malloc_stats() and having it print
> the relevent malloc data, on x86-64.  Between that and direct review of
> the patch, is it appropriate to jump through these hoops for a third
> layer of checking, vs waiting until tunables has a way to test *every*
> tunable via a standard regression test framework?
> 
>> The only _real_ way to fix this is to expose a GLIBC_PRIVATE symbols
>> which allow access to get/set the tunables.
> 
> IMHO a tunables_get() framework, GLIBC_PRIVATE or otherwise, is a much
> better solution.  I will promise to add suitable regression tests at
> that point, for malloc, if someone reminds me then ;-)

I can agree to that.

Please commit your patch as-is and we'll work on the regression framework
requirements when 2.26 opens.

Thank you again for converting tcache to the new tunables, and testing
the SIZE_T support.

-- 
Cheers,
Carlos.