This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] powerpc: fix sysconf support for cache geometries
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> On 20/06/2017 18:08, Tulio Magno Quites Machado Filho wrote:
>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>
>>> On 16/06/2017 10:36, Tulio Magno Quites Machado Filho wrote:
>>>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>>>
>>>>> On 16/06/2017 09:00, Paul Clarke wrote:
>>>>>> On 06/15/2017 03:55 PM, Adhemerval Zanella wrote:
>>>>>>> On 15/06/2017 17:45, Paul Clarke wrote:
>>>>>>>> Commit cdfbe5037f2f67bf5f560b73732b69d0fabe2314 added sysconf support
>>>>>>>> for cache geometries on powerpc, but mishandled errno. For valid input
>>>>>>>> parameters, sysconf() should not set errno.
>>>>>>>
>>>>>>> Do we have a testcase for it? If not I think we should add it.
>>>>>>
>>>>>> We don't. There are three obvious test cases for sysconf:
>>>>>> - ./nptl/tst-sysconf.c: ignores errno
>>>>>> - ./posix/tst-sysconf.c: ignores errno
>>>>>> - ./sysdeps/unix/sysv/linux/powerpc/tst-sysconf.c
>>>>>> Ironically (given the need for the subject patch), this correctly tests for errno changes but only for the _SC_LEVEL*, and only for powerpc.
>>>>>
>>>>> I think you might have a staged change in your repo, I do not see
>>>>> the powerpc specific one in master. In any case, I think it worth
>>>>> to add some errno testing on nptl and posix tests (maybe even
>>>>> incorporate nptl on posix) and add powerpc one (be aware to avoid
>>>>> add a testcase with same name in same folder, as this powerpc seems
>>>>> to be doing).
>>>>
>>>> I guess Paul meant to say
>>>> sysdeps/unix/sysv/linux/powerpc/test-powerpc-linux-sysconf.c, which is the
>>>> new name of the test after solving the name clash issue. ;-)
>>>
>>> Right, so that's the one that caused some trouble in the past. We can use
>>> this file to add a regression test then.
>>
>> I was planning to push this patch, but your last message wasn't entirely clear
>> to me.
>> Do you think there is something else to be done in this patch?
>>
>
> My understanding based on Paul's answer is we do not have regression tests
> for the very issues this patch intend, but test-powerpc-linux-sysconf.c does
> seem to have tests for errno handling (which may only be trigger in recent
> kernel due underlying support). If it is the case the patch lgtm, otherwise
> I think we need to add some regression tests.
I confirmed that's indeed the case, so I'm pushing this patch.
--
Tulio Magno