Re: [LTP] [PATCH] lib: tst_kconfig: Add runtime checks

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril, Li,

Thanks!

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Few notes below.

> So far for CONFIG_*_NS.

> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  lib/newlib_tests/test_kconfig.c |  1 +
>  lib/tst_kconfig.c               | 35 +++++++++++++++++++++++++++++++++
>  lib/tst_ns_checks.h             | 32 ++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+)
>  create mode 100644 lib/tst_ns_checks.h

> diff --git a/lib/newlib_tests/test_kconfig.c b/lib/newlib_tests/test_kconfig.c
> index cea36b5ee..ed2c4610a 100644
> --- a/lib/newlib_tests/test_kconfig.c
> +++ b/lib/newlib_tests/test_kconfig.c
> @@ -18,6 +18,7 @@ static const char *kconfigs[] = {
>  	"CONFIG_MMU & CONFIG_EXT4_FS=m",
>  	"CONFIG_EXT4_FS=m | CONFIG_MMU",
>  	"CONFIG_DEFAULT_HOSTNAME=\"(none)\"",
> +	"CONFIG_USER_NS",
>  	NULL
>  };

> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 9bcd57721..8d0f8ae3a 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -16,6 +16,8 @@
>  #include "tst_bool_expr.h"
>  #include "tst_safe_stdio.h"

> +#include "tst_ns_checks.h"
nit: don't we want to have tst_kconfig_checks.h which would have all configs?
Sure it can be this way, but I tend to have less files with more content.
I would be even ok with having everything in tst_kconfig.c, but understand you
don't want it.

> +
>  static int kconfig_skip_check(void)
>  {
>  	char *skipped = getenv("KCONFIG_SKIP_CHECK");
> @@ -110,6 +112,37 @@ static void close_kconfig(FILE *fp)
>  		fclose(fp);
>  }

> +static struct runtime_check {
> +	const char *config;
> +	bool (*runtime_check)(void);
> +} runtime_checks[] = {
> +	{"CONFIG_USER_NS", tst_user_ns_enabled},
> +	{"CONFIG_NET_NS", tst_net_ns_enabled},
> +	{"CONFIG_PID_NS", tst_pid_ns_enabled},
> +	{"CONFIG_MNT_NS", tst_mnt_ns_enabled},
> +	{"CONFIG_IPC_NS", tst_ipc_ns_enabled},
> +	{}
> +};
> +
> +static void runtime_check(struct tst_kconfig_var *var)
> +{
> +	size_t i;
> +
> +	for (i = 0; runtime_checks[i].config; i++) {
> +		if (strcmp(runtime_checks[i].config, var->id))
> +			continue;
> +
> +		tst_res(TDEBUG, "Running runtime check for '%s'", var->id);
This will not work since Li's change:
aa5a6fcdcd ("lib: suppress early TDEBUG output before context initialization")

@Li I'm not sure what "unless explicitly enabled" means, but I guess we cannot
simple enable it for the test library (following patch). I vote to either revert
aa5a6fcdcd or change it (effectively revert it, but keep doc and the rest of the
code).

I understand having the output in each test is not ideal:

utsname01.c:39: TDEBUG: mmap((nil), 64, PROT_READ | PROT_WRITE(3), 33, -1, 0)
utsname01.c:40: TDEBUG: mmap((nil), 64, PROT_READ | PROT_WRITE(3), 33, -1, 0)

but better more output code than no code.

+++ lib/tst_test.c
@@ -492,16 +492,12 @@ void tst_res_(const char *file, const int lineno, int ttype,
 	/*
 	 * Suppress TDEBUG output in these cases:
 	 * 1. No context available (e.g., called before IPC initialization)
-	 * 2. Called from the library process, unless explicitly enabled
-	 * 3. Debug output is not enabled (context->tdebug == 0)
+	 * 2. Debug output is not enabled (context->tdebug == 0)
 	 */
 	if (ttype == TDEBUG) {
 		if (!context)
 			return;
 
-		if (context->lib_pid == getpid())
-			return;
-
 		if (!context->tdebug)
 			return;
 	}

> +
> +		if (!runtime_checks[i].runtime_check()) {
> +			tst_res(TINFO,
> +				"%s=%c present but disabled at runtime",
> +				var->id, var->choice);
> +			var->choice = 'n';
> +		}
> +	}
> +}
> +
>  static inline int kconfig_parse_line(const char *line,
>                                       struct tst_kconfig_var *vars,
>                                       unsigned int vars_len)
> @@ -183,9 +216,11 @@ out:
>  			switch (val[0]) {
>  			case 'y':
>  				vars[i].choice = 'y';
> +				runtime_check(&vars[i]);
>  				return 1;
>  			case 'm':
>  				vars[i].choice = 'm';
> +				runtime_check(&vars[i]);
>  				return 1;
>  			}
>  		}
> diff --git a/lib/tst_ns_checks.h b/lib/tst_ns_checks.h
> new file mode 100644
> index 000000000..743d3d09d
> --- /dev/null
> +++ b/lib/tst_ns_checks.h
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
nit: please use /* */ otherwise checkpatch complains.
/* SPDX-License-Identifier: GPL-2.0-or-later */

Kind regards,
Petr

> +/*
> + * Copyright (c) 2026 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +#include <unistd.h>
> +#include <stdbool.h>
> +
> +static inline bool tst_user_ns_enabled(void)
> +{
> +	return access("/proc/self/ns/user", F_OK) == 0;
> +}
> +
> +static inline bool tst_net_ns_enabled(void)
> +{
> +	return access("/proc/self/ns/net", F_OK) == 0;
> +}
> +
> +static inline bool tst_pid_ns_enabled(void)
> +{
> +	return access("/proc/self/ns/pid", F_OK) == 0;
> +}
> +
> +static inline bool tst_mnt_ns_enabled(void)
> +{
> +	return access("/proc/self/ns/mnt", F_OK) == 0;
> +}
> +
> +static inline bool tst_ipc_ns_enabled(void)
> +{
> +	return access("/proc/self/ns/ipc", F_OK) == 0;
> +}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp