Re: [PATCH can-next v2 2/9] selftests: can: add J1939 test helper header and library

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests: can: silent skip in j1939_test_main() when protocol is missing
--

commit f577ac7f70eba0a6d39e0839d07a59a287fefd8a
Author: Oleksij Rempel <o.rempel@pengutronix.de>

selftests: can: add J1939 test helper header and library

This commit adds j1939_tests.h and j1939_lib.sh to provide shared C
helpers and vcan setup/teardown logic for the J1939 selftests.

> diff --git a/tools/testing/selftests/net/can/j1939_tests.h b/tools/testing/selftests/net/can/j1939_tests.h
> new file mode 100644
> index 0000000000000..78eb35f5b6b06
> --- /dev/null
> +++ b/tools/testing/selftests/net/can/j1939_tests.h
> @@ -0,0 +1,724 @@

[ ... ]

> +/* Is the CAN protocol proto missing from this kernel? */
> +static inline bool can_proto_missing(int type, int proto)
> +{
> +	int probe = socket(PF_CAN, type, proto);
> +
> +	if (probe < 0)
> +		return errno == EAFNOSUPPORT || errno == EPROTONOSUPPORT;
> +
> +	close(probe);
> +	return false;
> +}
> +
> +/*
> + * Body of each test's main(): require CANIF in the environment and a kernel
> + * that supports CAN_J1939 and CAN_RAW, then run the harness. Skip when any
> + * of them is missing.
> + */
> +static inline int j1939_test_main(int argc, char **argv)
> +{
> +	char *ifname = getenv("CANIF");
> +
> +	if (!ifname) {
> +		fprintf(stderr, "# CANIF must name the test interface\n");
> +		return KSFT_SKIP;
> +	}
> +	strncpy(CANIF, ifname, sizeof(CANIF) - 1);
> +
> +	if (can_proto_missing(SOCK_DGRAM, CAN_J1939) ||
> +	    can_proto_missing(SOCK_RAW, CAN_RAW))
> +		return KSFT_SKIP;

[Severity: Low]
In tools/testing/selftests/net/can/j1939_tests.h, when can_proto_missing()
returns true because socket(PF_CAN, SOCK_DGRAM, CAN_J1939) or
socket(PF_CAN, SOCK_RAW, CAN_RAW) fails with EAFNOSUPPORT or
EPROTONOSUPPORT, j1939_test_main() returns KSFT_SKIP without printing why
the test was skipped.

Since j1939_run() in tools/testing/selftests/net/can/j1939_lib.sh only
passes J1939_TEST_PROG to ret_set_ksft_status on a skip exit code, could
j1939_test_main() print a message stating which protocol support is missing
(similar to the CANIF check right above it) so the skip reason is visible
in the test output?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-j1939-selftests-v2-0-cf797c83317b@pengutronix.de?part=2