linux-kernel - Re: [PATCH] selftests: lib: Skip tests on missing test modules

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] [day] [month] [year] [list]

Message-ID: <7cd2f051-6bef-db88-85d6-a8c49e00dece@kernel.org>
Date:   Fri, 16 Jun 2017 10:01:18 -0600
From:   Shuah Khan <shuah@...nel.org>
To:     Sumit Semwal <sumit.semwal@...aro.org>,
        linux-kselftest@...r.kernel.org
Cc:     keescook@...omium.org, decot@...glers.com,
        linux-kernel@...r.kernel.org, Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH] selftests: lib: Skip tests on missing test modules

On 06/16/2017 09:10 AM, Shuah Khan wrote:
> Hi Sumit,
> 
> On 06/15/2017 11:29 PM, Sumit Semwal wrote:
>> With older kernels, printf.sh and bitmap.sh fail because they can't find
>> the respective test modules they are looking for.
>>
>> Add the skip portion on missing the respective test_XXX module. Error out
>> the same way as prime_numbers.sh.
>>
>> Signed-off-by: Sumit Semwal <sumit.semwal@...aro.org>
>> ---
>>  tools/testing/selftests/lib/bitmap.sh | 5 +++++
>>  tools/testing/selftests/lib/printf.sh | 5 +++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh
>> index 2da187b6ddad..85294b4a0861 100755
>> --- a/tools/testing/selftests/lib/bitmap.sh
>> +++ b/tools/testing/selftests/lib/bitmap.sh
>> @@ -1,6 +1,11 @@
>>  #!/bin/sh
>>  # Runs bitmap infrastructure tests using test_bitmap kernel module
>>  
>> +if ! /sbin/modprobe -q test_bitmap; then
>> +        echo "bitmap: [SKIP]"
>> +	exit 77
>> +fi
> 
> Please add this logic below in the else clause instead of
> adding duplicate block.
> 
> I think it is would be good to differentiate when a modules
> isn't present vs. modprobe failed keying off of
> 
> "Module test_printf not found" message.
> 
> The second one can be separate patch.

Please make the change to detect module not present vs.
module failed load in the same patch. Thinking about more,
this change will always report SKIP which is worse than
what we had before that it always reports FAIL.

The reason being, reporting SKIP when module is present and
failed load due to some error is worse than reporting FAIL
when module isn't present.

> 
>> +
>>  if /sbin/modprobe -q test_bitmap; then
>>  	/sbin/modprobe -q -r test_bitmap
>>  	echo "bitmap: ok"
>> diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh
>> index 4fdc70fe6980..024e749a83d4 100755
>> --- a/tools/testing/selftests/lib/printf.sh
>> +++ b/tools/testing/selftests/lib/printf.sh
>> @@ -1,6 +1,11 @@
>>  #!/bin/sh
>>  # Runs printf infrastructure using test_printf kernel module
>>  
>> +if ! /sbin/modprobe -q test_printf; then
>> +        echo "printf: [SKIP]"
>> +	exit 77
>> +fi
> 
> Please add this logic below in the else clause instead of
> adding duplicate block.
> 
> I think it is would be good to differentiate when a modules
> isn't present vs. modprobe failed keying off of
> 
> "Module test_printf not found" message.
> 
> The second one can be separate patch.

Same commnet as above.

> 
>> +
>>  if /sbin/modprobe -q test_printf; then
>>  	/sbin/modprobe -q -r test_printf
>>  	echo "printf: ok"
>>
> 
> thanks,
> -- Shuah
> 
> 

thanks,
-- Shuah