lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3408117a-ae15-48f3-9762-ef02b7460851@collabora.com>
Date: Tue, 2 Jul 2024 15:16:58 +0500
From: Muhammad Usama Anjum <usama.anjum@...labora.com>
To: Shuah Khan <skhan@...uxfoundation.org>, Shuah Khan <shuah@...nel.org>,
 Vincenzo Frascino <vincenzo.frascino@....com>,
 Tiezhu Yang <yangtiezhu@...ngson.cn>, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: Muhammad Usama Anjum <usama.anjum@...labora.com>, kernel@...labora.com
Subject: Re: [PATCH v2 1/4] kselftests: vdso: vdso_test_clock_getres: conform
 test to TAP output

Kind reminder to review

On 6/12/24 1:17 PM, Muhammad Usama Anjum wrote:
> On 6/12/24 1:32 AM, Shuah Khan wrote:
>> On 6/9/24 23:41, Muhammad Usama Anjum wrote:
>>> Conform the layout, informational and status messages to TAP. No
>>> functional change is intended other than the layout of output messages.
>>> Use kselftest_harness.h to conform to TAP as the number of tests depend
>>> on the available options at build time. The kselftest_harness makes the
>>
>>
>> How does converting to kselftest_harness help with available options ay
>> build time? Can you explain?
>>
>> I am not seeing any value in converting this test to the harness? I want
>> to see a better justification.
> 
> Before:
> ./vdso_test_clock_getres
> clock_id: CLOCK_REALTIME [PASS]
> clock_id: CLOCK_BOOTTIME [PASS]
> clock_id: CLOCK_TAI [PASS]
> clock_id: CLOCK_REALTIME_COARSE [PASS]
> clock_id: CLOCK_MONOTONIC [PASS]
> clock_id: CLOCK_MONOTONIC_RAW [PASS]
> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
> 
> Here is the output of the test before this patch. The test output test
> names and if they are passed or failed. It doesn't output information
> related to error when it occurs. I wanted to convert it to standard format
> by using kselftest.h where we can get the error related information as
> well. But as the number of tests depend on how many of CLOCK_BOOTTIME,
> CLOCK_TAI etc are defined, static counting is difficult. Test harness is
> best suited for this. Output:
> 
> ./vdso_test_clock_getres
> TAP version 13
> 1..7
> # Starting 7 tests from 1 test cases.
> #  RUN           global.clock_realtime ...
> #            OK  global.clock_realtime
> ok 1 global.clock_realtime
> #  RUN           global.clock_boottime ...
> #            OK  global.clock_boottime
> ok 2 global.clock_boottime
> #  RUN           global.clock_tai ...
> #            OK  global.clock_tai
> ok 3 global.clock_tai
> #  RUN           global.clock_realtime_coarse ...
> #            OK  global.clock_realtime_coarse
> ok 4 global.clock_realtime_coarse
> #  RUN           global.clock_monotonic ...
> #            OK  global.clock_monotonic
> ok 5 global.clock_monotonic
> #  RUN           global.clock_monotonic_raw ...
> #            OK  global.clock_monotonic_raw
> ok 6 global.clock_monotonic_raw
> #  RUN           global.clock_monotonic_coarse ...
> #            OK  global.clock_monotonic_coarse
> ok 7 global.clock_monotonic_coarse
> # PASSED: 7 / 7 tests passed.
> # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Not only the code is simplified, the descriptive error is printed on
> console that what went wrong. Example if a test case fails:
> 
> #  RUN           global.clock_realtime ...
> # vdso_test_clock_getres.c:66:clock_realtime:Expected 1 (1) == ((x.tv_sec
> != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) (0)
> # clock_realtime: Test terminated by assertion
> #          FAIL  global.clock_realtime
> not ok 1 global.clock_realtime
> 
>>
>>> test easy to convert and presents better maintainability.
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
>>> ---
>>> Changes since v1:
>>> - Update commit message to include that kselftest_harness has been used
>>>    to conform to TAP and why
>>> ---
>>>   .../selftests/vDSO/vdso_test_clock_getres.c   | 68 +++++++++----------
>>>   1 file changed, 33 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>>> b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>>> index 38d46a8bf7cba..c1ede40521f05 100644
>>> --- a/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>>> +++ b/tools/testing/selftests/vDSO/vdso_test_clock_getres.c
>>> @@ -25,7 +25,7 @@
>>>   #include <unistd.h>
>>>   #include <sys/syscall.h>
>>>   -#include "../kselftest.h"
>>> +#include "../kselftest_harness.h"
>>>     static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
>>>   {
>>> @@ -54,18 +54,8 @@ const char *vdso_clock_name[12] = {
>>>   /*
>>>    * This function calls clock_getres in vdso and by system call
>>>    * with different values for clock_id.
>>> - *
>>> - * Example of output:
>>> - *
>>> - * clock_id: CLOCK_REALTIME [PASS]
>>> - * clock_id: CLOCK_BOOTTIME [PASS]
>>> - * clock_id: CLOCK_TAI [PASS]
>>> - * clock_id: CLOCK_REALTIME_COARSE [PASS]
>>> - * clock_id: CLOCK_MONOTONIC [PASS]
>>> - * clock_id: CLOCK_MONOTONIC_RAW [PASS]
>>> - * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>>    */
>>> -static inline int vdso_test_clock(unsigned int clock_id)
>>> +static inline void vdso_test_clock(struct __test_metadata *_metadata,
>>> unsigned int clock_id)
>>>   {
>>>       struct timespec x, y;
>>>   @@ -73,52 +63,60 @@ static inline int vdso_test_clock(unsigned int
>>> clock_id)
>>>       clock_getres(clock_id, &x);
>>>       syscall_clock_getres(clock_id, &y);
>>>   -    if ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)) {
>>> -        printf(" [FAIL]\n");
>>> -        return KSFT_FAIL;
>>> -    }
>>> -
>>> -    printf(" [PASS]\n");
>>> -    return KSFT_PASS;
>>> +    ASSERT_EQ(0, ((x.tv_sec != y.tv_sec) || (x.tv_nsec != y.tv_nsec)));
>>>   }
>>>   -int main(int argc, char **argv)
>>> -{
>>> -    int ret = 0;
>>> -
>>>   #if _POSIX_TIMERS > 0
>>>     #ifdef CLOCK_REALTIME
>>> -    ret += vdso_test_clock(CLOCK_REALTIME);
>>> +TEST(clock_realtime)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_REALTIME);
>>> +}
>>>   #endif
>>>     #ifdef CLOCK_BOOTTIME
>>> -    ret += vdso_test_clock(CLOCK_BOOTTIME);
>>> +TEST(clock_boottime)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_BOOTTIME);
>>> +}
>>>   #endif
>>>     #ifdef CLOCK_TAI
>>> -    ret += vdso_test_clock(CLOCK_TAI);
>>> +TEST(clock_tai)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_TAI);
>>> +}
>>>   #endif
>>>     #ifdef CLOCK_REALTIME_COARSE
>>> -    ret += vdso_test_clock(CLOCK_REALTIME_COARSE);
>>> +TEST(clock_realtime_coarse)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_REALTIME_COARSE);
>>> +}
>>>   #endif
>>>     #ifdef CLOCK_MONOTONIC
>>> -    ret += vdso_test_clock(CLOCK_MONOTONIC);
>>> +TEST(clock_monotonic)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_MONOTONIC);
>>> +}
>>>   #endif
>>>     #ifdef CLOCK_MONOTONIC_RAW
>>> -    ret += vdso_test_clock(CLOCK_MONOTONIC_RAW);
>>> +TEST(clock_monotonic_raw)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_MONOTONIC_RAW);
>>> +}
>>>   #endif
>>>     #ifdef CLOCK_MONOTONIC_COARSE
>>> -    ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE);
>>> +TEST(clock_monotonic_coarse)
>>> +{
>>> +    vdso_test_clock(_metadata, CLOCK_MONOTONIC_COARSE);
>>> +}
>>>   #endif
>>>   -#endif
>>> -    if (ret > 0)
>>> -        return KSFT_FAIL;
>>> +#endif /* _POSIX_TIMERS > 0 */
>>>   -    return KSFT_PASS;
>>> -}
>>> +TEST_HARNESS_MAIN
>>
>> thanks,
>> -- Shuah
>>
>>
> 

-- 
BR,
Muhammad Usama Anjum

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ