Re: [PATCH v14 14/15] barrier: add tests for smp_cond_load_*_timeout()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The string pointers `clk` and `update` in `param_to_desc()` may be left uninitialized.
- [Low] Typo in Kconfig help text for `BARRIER_TIMEOUT_KUNIT_TEST`: 'unusure' instead of 'unsure'.
--

--- Patch [14]: [PATCH v14 14/15] barrier: add tests for smp_cond_load_*_timeout() ---
commit a30c2efa70f8be2b1654f46db86c3a29fe02abf2
Author: Ankur Arora <ankur.a.arora@oracle.com>

barrier: add tests for smp_cond_load_*_timeout()

Add success and failure case tests for smp_cond_load_*_timeout().
Success or failure cases depend on the expected bit being set (or not).
Additionally in failure cases smp_cond_load_*_timeout() cannot return
before timeout.

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 8ff5adcfe1e0a..ce75655863f49 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
[ ... ]
> @@ -2551,6 +2551,16 @@ config FFS_KUNIT_TEST
>  	  For more information on KUnit and unit tests in general,
>  	  please refer to Documentation/dev-tools/kunit/.
>  
> +config BARRIER_TIMEOUT_KUNIT_TEST
> +	tristate "KUnit tests for smp_cond_load_relaxed_timeout()" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  Builds KUnit tests that validate wake-up and timeout handling paths
> +	  in smp_cond_load_relaxed_timeout().
> +
> +	  If unusure, say N.

[Severity: Low]
This isn't a bug, but there's a typo (unusure instead of unsure) in the help
text.

> diff --git a/lib/tests/barrier-timeout-test.c b/lib/tests/barrier-timeout-test.c
> new file mode 100644
> index 0000000000000..4226fb6456b35
> --- /dev/null
> +++ b/lib/tests/barrier-timeout-test.c
[ ... ]
> +static void param_to_desc(const struct smp_cond_update_params *p, char *desc)
> +{
> +	char *clk, *update;
> +
> +	if (p->clock == &mocked_clock) {
> +		clk = "mocked";
> +		update = "inline";
> +	} else if (p->clock == &basic_clock) {
> +		clk = "basic";
> +		update = "none";
> +	}
> +
> +
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "smp_cond_%s_timeout: clock-%s, update=%s",
> +		p->acquire ? "acquire" : "relaxed", clk, update);

[Severity: Low]
Could the string pointers clk and update be left uninitialized here?

If a different clock function is ever added and passed into param_to_desc,
the conditional chain will fall through without assigning these pointers.
This would result in passing uninitialized variables to snprintf, and might
also trigger -Wmaybe-uninitialized compiler warnings.

Would it be safer to add a default else branch or initialize these variables
to a fallback string like "unknown"?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714073041.40250-1-ankur.a.arora@oracle.com?part=14