Re: [PATCH] selftests: Improves rtctest error handling.

[Alexandre Belloni via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org>]

On 08/04/2023 13:10:59+0530, Atul Kumar Pant wrote:
> When running the rtctest without root privileges the test fails
> expectedly, but prints the logs that are not useful to point to the issue.
> Similarly, if we pass wrong rtc device file as an argument, the test
> output failure logs do not point to the issue that the rtc file is
> invalid.
> To handle these issues, this patch adds checks to verify uid with which
> the test is run and also if the rtc_file is valid.
> 
> Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> ---
>  tools/testing/selftests/rtc/rtctest.c | 34 +++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 10 deletions(-)
> 

Please run your patch through checkpatch.pl, the indentation and comment
style are not correct.

> diff --git a/tools/testing/selftests/rtc/rtctest.c b/tools/testing/selftests/rtc/rtctest.c
> index 2b9d929a24ed..9564346c63eb 100644
> --- a/tools/testing/selftests/rtc/rtctest.c
> +++ b/tools/testing/selftests/rtc/rtctest.c
> @@ -388,16 +388,30 @@ __constructor_order_last(void)
>  
>  int main(int argc, char **argv)
>  {
> -	switch (argc) {
> -	case 2:
> -		rtc_file = argv[1];
> -		/* FALLTHROUGH */
> -	case 1:
> -		break;
> -	default:
> -		fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]);
> -		return 1;
> +    int ret = -1;
> +
> +    // Verify if the test is run as root
> +    if (getuid() != 0) {
> +		ksft_exit_skip("[ERROR]: Please run the test as root - Exiting.\n");

If this is an error, should we really return skip? This may end up being
silently ignored whereas the expectation is to test the rtc.

Also, you may use the rtc as non root so I guess the access() test
should be enough.

> +    }
> +
> +    switch (argc) {
> +    case 2:
> +        rtc_file = argv[1];
> +        /* FALLTHROUGH */
> +    case 1:
> +        break;
> +    default:
> +        fprintf(stderr, "usage: %s [rtcdev]\n", argv[0]);
> +        return 1;
>  	}
>  
> -	return test_harness_run(argc, argv);
> +    // Run the test if rtc_file is valid
> +    if (access(rtc_file, F_OK) == 0) {
> +	    ret = test_harness_run(argc, argv);
> +    } else {
> +		ksft_exit_skip("[ERROR]: %s : File does not exists - Exiting\n", rtc_file);


> +    }
> +
> +    return ret;
>  }
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees