Re: [PATCH v2] selftests: rtc: Support harness test filtering and arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: rtc: incomplete getopt() reset on -h/-l in main()
--

commit 9bed5ee7eee853de78b5dce8eb5280f5ed1c8688
Author: Wake Liu <wakel@google.com>

selftests: rtc: Support harness test filtering and arguments

This commit updates main() in rtctest.c to parse command-line options with
getopt() so kselftest harness flags can be passed alongside the optional RTC
device node path.

> diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
> index 552191fe1ca09..eb26d957835d7 100644
> --- a/tools/testing/selftests/rtc/rtctest.c
> +++ b/tools/testing/selftests/rtc/rtctest.c
> @@ -487,18 +487,44 @@ TEST_F_TIMEOUT(rtc, alarm_wkalm_set_minute, 65) {
>  int main(int argc, char **argv)
>  {
>  	int ret = -1;
> +	int opt, i;
> +	int non_opt = 0;
> +
> +	while ((opt = getopt(argc, argv, "dhlF:f:V:v:t:T:r:")) != -1) {
> +		switch (opt) {
> +		case 'h':
> +			fprintf(stderr,
> +				"Usage: %s [-h|-l|-d] [-t|-T|-v|-V|-f|-F|-r name] [rtcdev]\n\n",
> +				argv[0]);
> +			optind = 1;
> +			return test_harness_run(argc, argv);
> +		case 'l':
> +			optind = 1;
> +			return test_harness_run(argc, argv);

[Severity: Medium]
What happens if -h or -l is passed as the first flag in a clustered short
option argument (such as -hd or -ld)?

When getopt() parses the first character in argv[1], optind is still 1 and
getopt()'s internal within-argument character cursor points to the second
character ('d'). Returning early from inside the getopt() loop after setting
optind = 1 does not reset that internal cursor when getopt() has not finished
consuming the current argument.

When main() calls test_harness_run() -> test_harness_argv_check():

tools/testing/selftests/kselftest_harness.h:test_harness_argv_check() {
    ...
    while ((opt = getopt(argc, argv, "dhlF:f:V:v:t:T:r:")) != -1) {
        switch (opt) {
        ...
        case 'h':
            test_harness_usage(argv[0]);
            return KSFT_FAIL;
        case 'l':
            __test_harness_list_tests();
            return KSFT_FAIL;
        ...
        }
    }
    return KSFT_PASS;
}

getopt() in test_harness_argv_check() resumes at 'd' instead of re-reading
'h' or 'l', returns KSFT_PASS, and runs the full RTC test suite (while also
having bypassed the access(rtc_file, R_OK) check in main()).

Would it be better to let the getopt() loop finish scanning all options (for
example by recording a flag for -h/-l or resetting optind = 0 where supported)
before calling test_harness_run()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105030834.3318480-1-wakel@google.com?part=1