| 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
| ||
|
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