This is the mail archive of the 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] powerpc: fix sysconf support for cache geometries

On 20/06/2017 18:08, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <> writes:
>> On 16/06/2017 10:36, Tulio Magno Quites Machado Filho wrote:
>>> Adhemerval Zanella <> 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.

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