Re: [PATCH v3] lib/glob: convert selftest to KUnit

[Kuan-Wei Chiu <visitorckw@gmail.com>]

Hi Kir,

On Wed, Jan 07, 2026 at 02:19:03PM +0900, Kir Chou wrote:
> This patch converts the existing glob selftest (lib/globtest.c) to use
> the KUnit framework (lib/tests/glob_kunit.c).
> 
> The new test:
> 
> - Migrates all 64 test cases from the original test to the KUnit suite.
> - Removes the custom 'verbose' module parameter as KUnit handles logging.
> - Updates Kconfig.debug and Makefile to support the new KUnit test.
> - Updates Kconfig and Makefile to remove the original selftest.
> - Updates GLOB_SELFTEST to GLOB_KUNIT_TEST for arch/m68k/configs.
> 
> This commit is verified by `./tools/testing/kunit/kunit.py run`
> with the .kunit/.kunitconfig:
> 
> ```
> CONFIG_KUNIT=y
> CONFIG_GLOB_KUNIT_TEST=y
> ```
> 
> Signed-off-by: Kir Chou <note351@hotmail.com>

Thanks for the patch.

However, it seems you missed carrying forward David's Reviewed-by and
Geert's Acked-by tags. Please include them in the next version. If you
dropped them intentionally (e.g., due to changes in v3), please explain
why under --- line.

> ---
> v2:
>  - Remove CONFIG_GLOB_KUNIT_TEST from defconfigs as it's implicitly enabled
>    by CONFIG_KUNIT_ALL_TESTS. (Suggested by Geert)
> v3:
>  - Move lib/glob_kunit.c to lib/tests/glob_kunit.c.
>  - Update Makefile accordingly.
> ---
>  arch/m68k/configs/amiga_defconfig    |   1 -
>  arch/m68k/configs/apollo_defconfig   |   1 -
>  arch/m68k/configs/atari_defconfig    |   1 -
>  arch/m68k/configs/bvme6000_defconfig |   1 -
>  arch/m68k/configs/hp300_defconfig    |   1 -
>  arch/m68k/configs/mac_defconfig      |   1 -
>  arch/m68k/configs/multi_defconfig    |   1 -
>  arch/m68k/configs/mvme147_defconfig  |   1 -
>  arch/m68k/configs/mvme16x_defconfig  |   1 -
>  arch/m68k/configs/q40_defconfig      |   1 -
>  arch/m68k/configs/sun3_defconfig     |   1 -
>  arch/m68k/configs/sun3x_defconfig    |   1 -
>  lib/Kconfig                          |  13 ---
>  lib/Kconfig.debug                    |  13 +++
>  lib/Makefile                         |   1 -
>  lib/globtest.c                       | 167 ---------------------------
>  lib/tests/Makefile                   |   1 +
>  lib/tests/glob_kunit.c               | 122 +++++++++++++++++++
>  18 files changed, 136 insertions(+), 193 deletions(-)
>  delete mode 100644 lib/globtest.c
>  create mode 100644 lib/tests/glob_kunit.c
> 

...

> @@ -430,19 +430,6 @@ config GLOB
>  	  are compiling an out-of tree driver which tells you that it
>  	  depends on this.
>  
> -config GLOB_SELFTEST
> -	tristate "glob self-test on init"
> -	depends on GLOB
> -	help
> -	  This option enables a simple self-test of the glob_match
> -	  function on startup.	It is primarily useful for people
> -	  working on the code to ensure they haven't introduced any
> -	  regressions.
> -
> -	  It only adds a little bit of code and slows kernel boot (or
> -	  module load) by a small amount, so you're welcome to play with
> -	  it, but you probably don't need it.
> -
>  #
>  # Netlink attribute parsing support is select'ed if needed
>  #
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index ba36939fd..e4347e145 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -3354,6 +3354,19 @@ config PRIME_NUMBERS_KUNIT_TEST
>  
>  	  If unsure, say N
>  
> +config GLOB_KUNIT_TEST
> +	tristate "Glob matching test" if !KUNIT_ALL_TESTS
> +	depends on KUNIT
> +	default KUNIT_ALL_TESTS
> +	select GLOB

The original test used depends on GLOB.
Any specific reason for switching to select GLOB?
Ideally, tests should not select functionality, to prevent increasing
the attack vector.

> +	help
> +	  Enable this option to test the glob functions at runtime.
> +
> +	  This test suite verifies the correctness of glob_match() across various
> +	  scenarios, including edge cases.
> +
> +	  If unsure, say N
> +
>  endif # RUNTIME_TESTING_MENU
>  
>  config ARCH_USE_MEMTEST

...

> diff --git a/lib/tests/glob_kunit.c b/lib/tests/glob_kunit.c
> new file mode 100644
> index 000000000..797e32a8c
> --- /dev/null
> +++ b/lib/tests/glob_kunit.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0

The original file lib/globtest.c (and lib/glob.c) is dual-licensed
(MIT/GPL), and your MODULE_LICENSE at the end of this file also states
"Dual MIT/GPL".

However, this SPDX identifier marks the file as GPL-2.0 only.

> +/*
> + * Test cases for glob functions.
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/glob.h>
> +#include <linux/module.h>
> +
> +struct glob_test_case {
> +	// Pattern to match.
> +	const char *pat;
> +	// String to match against.
> +	const char *str;
> +	// Expected glob_match result, true is matched.

If you want to document the struct members, the standard kernel-doc
format [1] would be preferred over C++-style comments.

[1]: https://origin.kernel.org/doc/html/v6.18/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

> +	bool expected;
> +};
> +
> +static const struct glob_test_case glob_test_cases[] = {
> +	/* Some basic tests */
> +	{ .pat = "a", .str = "a", .expected = true },
> +	{ .pat = "a", .str = "b", .expected = false },
> +	{ .pat = "a", .str = "aa", .expected = false },
> +	{ .pat = "a", .str = "", .expected = false },
> +	{ .pat = "", .str = "", .expected = true },
> +	{ .pat = "", .str = "a", .expected = false },
> +	/* Simple character class tests */
> +	{ .pat = "[a]", .str = "a", .expected = true },
> +	{ .pat = "[a]", .str = "b", .expected = false },
> +	{ .pat = "[!a]", .str = "a", .expected = false },
> +	{ .pat = "[!a]", .str = "b", .expected = true },
> +	{ .pat = "[ab]", .str = "a", .expected = true },
> +	{ .pat = "[ab]", .str = "b", .expected = true },
> +	{ .pat = "[ab]", .str = "c", .expected = false },
> +	{ .pat = "[!ab]", .str = "c", .expected = true },
> +	{ .pat = "[a-c]", .str = "b", .expected = true },
> +	{ .pat = "[a-c]", .str = "d", .expected = false },
> +	/* Corner cases in character class parsing */
> +	{ .pat = "[a-c-e-g]", .str = "-", .expected = true },
> +	{ .pat = "[a-c-e-g]", .str = "d", .expected = false },
> +	{ .pat = "[a-c-e-g]", .str = "f", .expected = true },
> +	{ .pat = "[]a-ceg-ik[]", .str = "a", .expected = true },
> +	{ .pat = "[]a-ceg-ik[]", .str = "]", .expected = true },
> +	{ .pat = "[]a-ceg-ik[]", .str = "[", .expected = true },
> +	{ .pat = "[]a-ceg-ik[]", .str = "h", .expected = true },
> +	{ .pat = "[]a-ceg-ik[]", .str = "f", .expected = false },
> +	{ .pat = "[!]a-ceg-ik[]", .str = "h", .expected = false },
> +	{ .pat = "[!]a-ceg-ik[]", .str = "]", .expected = false },
> +	{ .pat = "[!]a-ceg-ik[]", .str = "f", .expected = true },
> +	/* Simple wild cards */
> +	{ .pat = "?", .str = "a", .expected = true },
> +	{ .pat = "?", .str = "aa", .expected = false },
> +	{ .pat = "??", .str = "a", .expected = false },
> +	{ .pat = "?x?", .str = "axb", .expected = true },
> +	{ .pat = "?x?", .str = "abx", .expected = false },
> +	{ .pat = "?x?", .str = "xab", .expected = false },
> +	/* Asterisk wild cards (backtracking) */
> +	{ .pat = "*??", .str = "a", .expected = false },
> +	{ .pat = "*??", .str = "ab", .expected = true },
> +	{ .pat = "*??", .str = "abc", .expected = true },
> +	{ .pat = "*??", .str = "abcd", .expected = true },
> +	{ .pat = "??*", .str = "a", .expected = false },
> +	{ .pat = "??*", .str = "ab", .expected = true },
> +	{ .pat = "??*", .str = "abc", .expected = true },
> +	{ .pat = "??*", .str = "abcd", .expected = true },
> +	{ .pat = "?*?", .str = "a", .expected = false },
> +	{ .pat = "?*?", .str = "ab", .expected = true },
> +	{ .pat = "?*?", .str = "abc", .expected = true },
> +	{ .pat = "?*?", .str = "abcd", .expected = true },
> +	{ .pat = "*b", .str = "b", .expected = true },
> +	{ .pat = "*b", .str = "ab", .expected = true },
> +	{ .pat = "*b", .str = "ba", .expected = false },
> +	{ .pat = "*b", .str = "bb", .expected = true },
> +	{ .pat = "*b", .str = "abb", .expected = true },
> +	{ .pat = "*b", .str = "bab", .expected = true },
> +	{ .pat = "*bc", .str = "abbc", .expected = true },
> +	{ .pat = "*bc", .str = "bc", .expected = true },
> +	{ .pat = "*bc", .str = "bbc", .expected = true },
> +	{ .pat = "*bc", .str = "bcbc", .expected = true },
> +	/* Multiple asterisks (complex backtracking) */
> +	{ .pat = "*ac*", .str = "abacadaeafag", .expected = true },
> +	{ .pat = "*ac*ae*ag*", .str = "abacadaeafag", .expected = true },
> +	{ .pat = "*a*b*[bc]*[ef]*g*", .str = "abacadaeafag", .expected = true },
> +	{ .pat = "*a*b*[ef]*[cd]*g*", .str = "abacadaeafag", .expected = false },
> +	{ .pat = "*abcd*", .str = "abcabcabcabcdefg", .expected = true },
> +	{ .pat = "*ab*cd*", .str = "abcabcabcabcdefg", .expected = true },
> +	{ .pat = "*abcd*abcdef*", .str = "abcabcdabcdeabcdefg", .expected = true },
> +	{ .pat = "*abcd*", .str = "abcabcabcabcefg", .expected = false },
> +	{ .pat = "*ab*cd*", .str = "abcabcabcabcefg", .expected = false },
> +};
> +
> +static void glob_case_to_desc(const struct glob_test_case *t, char *desc)
> +{
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "pat:\"%s\" str:\"%s\"", t->pat, t->str);
> +}
> +
> +KUNIT_ARRAY_PARAM(glob, glob_test_cases, glob_case_to_desc);
> +
> +static void glob_test_match(struct kunit *test)
> +{
> +	const struct glob_test_case *params = test->param_value;
> +
> +	KUNIT_EXPECT_EQ_MSG(test,

checkpatch.pl reports an alignment issue here:

CHECK: Alignment should match open parenthesis
#632: FILE: lib/tests/glob_kunit.c:104:
+       KUNIT_EXPECT_EQ_MSG(test,
+               glob_match(params->pat, params->str),

Regards,
Kuan-Wei

> +		glob_match(params->pat, params->str),
> +		params->expected,
> +		"Pattern: \"%s\", String: \"%s\", Expected: %d",
> +		params->pat, params->str, params->expected);
> +}
> +
> +static struct kunit_case glob_kunit_test_cases[] = {
> +	KUNIT_CASE_PARAM(glob_test_match, glob_gen_params),
> +	{}
> +};
> +
> +static struct kunit_suite glob_test_suite = {
> +	.name = "glob",
> +	.test_cases = glob_kunit_test_cases,
> +};
> +
> +kunit_test_suite(glob_test_suite);
> +MODULE_DESCRIPTION("Test cases for glob functions");
> +MODULE_LICENSE("Dual MIT/GPL");
> -- 
> 2.52.0.351.gbe84eed79e-goog
> 
>