Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4] selftests/kselftest/runner.sh: Pass optional command
 parameters in environment
To:     Cristian Marussi <cristian.marussi@arm.com>,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     shuah@kernel.org, Kees Cook <keescook@chromium.org>,
        Shuah Khan <skhan@linuxfoundation.org>
References: <20220126103230.35285-1-cristian.marussi@arm.com>
From:   Shuah Khan <skhan@linuxfoundation.org>
Message-ID: <6905ff91-c363-91b3-7240-8bb728f45c39@linuxfoundation.org>
Date:   Thu, 27 Jan 2022 16:35:51 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <20220126103230.35285-1-cristian.marussi@arm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/26/22 3:32 AM, Cristian Marussi wrote:
> Some testcases allow for optional commandline parameters but as of now
> there is now way to provide such arguments to the runner script.
> 
> Add support to retrieve such optional command parameters fron environment
> variables named so as to include the all-uppercase test executable name,
> sanitized substituting any non-acceptable varname characters with "_",
> following the pattern:
> 
> 	KSELFTEST_<UPPERCASE_SANITIZED_TEST_NAME>_ARGS="options"
> 
> Optional command parameters support is not available if 'tr' is not
> installed on the test system.
> 
> Cc: Kees Cook <keescook@chromium.org>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - rebased on v5.17-rc1
> v2 --> v3
> - improved varname sanitation
> v1 --> v2
> - using env vars instead of settings file
> - added missing varname sanitation
> 
> Usage examples:
> 
> 	KSELFTEST_RTCTEST_ARGS="/dev/rtc1" /opt/ksft_fixes/run_kselftest.sh -c rtc
> 
> 	KSELFTEST_CPU_ON_OFF_TEST_SH_ARGS="-a -p 10" /opt/ksft_fixes/run_kselftest.sh -c cpu-hotplug
> ---
>   tools/testing/selftests/kselftest/runner.sh | 30 ++++++++++++++++++++-
>   1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index a9ba782d8ca0..294619ade49f 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -18,6 +18,8 @@ if [ -z "$BASE_DIR" ]; then
>   	exit 1
>   fi
>   
> +TR_CMD=$(command -v tr)
> +
>   # If Perl is unavailable, we must fall back to line-at-a-time prefixing
>   # with sed instead of unbuffered output.
>   tap_prefix()
> @@ -49,6 +51,31 @@ run_one()
>   
>   	# Reset any "settings"-file variables.
>   	export kselftest_timeout="$kselftest_default_timeout"
> +
> +	# Safe default if tr not available
> +	kselftest_cmd_args_ref="KSELFTEST_ARGS"
> +
> +	# Optional arguments for this command, possibly defined as an
> +	# environment variable built using the test executable in all
> +	# uppercase and sanitized substituting non acceptable shell
> +	# variable name characters with "_" as in:
> +	#
> +	# 	KSELFTEST_<UPPERCASE_SANITIZED_TESTNAME>_ARGS="<options>"
> +	#
> +	# e.g.
> +	#
> +	# 	rtctest --> KSELFTEST_RTCTEST_ARGS="/dev/rtc1"
> +	#
> +	# 	cpu-on-off-test.sh --> KSELFTEST_CPU_ON_OFF_TEST_SH_ARGS="-a -p 10"
> +	#
> +	if [ -n "$TR_CMD" ]; then
> +		BASENAME_SANITIZED=$(echo "$BASENAME_TEST" | \
> +					$TR_CMD -d "[:blank:][:cntrl:]" | \
> +					$TR_CMD -c "[:alnum:]_" "_" | \
> +					$TR_CMD [:lower:] [:upper:])
> +		kselftest_cmd_args_ref="KSELFTEST_${BASENAME_SANITIZED}_ARGS"
> +	fi
> +
>   	# Load per-test-directory kselftest "settings" file.
>   	settings="$BASE_DIR/$DIR/settings"
>   	if [ -r "$settings" ] ; then
> @@ -69,7 +96,8 @@ run_one()
>   		echo "# Warning: file $TEST is missing!"
>   		echo "not ok $test_num $TEST_HDR_MSG"
>   	else
> -		cmd="./$BASENAME_TEST"
> +		eval kselftest_cmd_args="\$${kselftest_cmd_args_ref:-}"
> +		cmd="./$BASENAME_TEST $kselftest_cmd_args"
>   		if [ ! -x "$TEST" ]; then
>   			echo "# Warning: file $TEST is not executable"
>   
> 

Looks good to me. Applying to linux-kselftest next for 5.18

thanks,
-- Shuah