Re: [PATCH v2] kconfig: warn about malformed KCONFIG_PROBABILITY values

[Julian Braha]

Hi Dmitrii,

On 9/8/26 21:36, Dmitrii Tulnov wrote:
> randconfig checks the numeric range of each probability but does not
> validate where strtol() stops. For example, KCONFIG_PROBABILITY=50% is
> accepted as 50:0:0: the '%' is parsed repeatedly as zero. This silently
> sets both tristate y/m probabilities to zero, reducing the coverage of
> random configuration builds. Empty fields and extra fields are also
> accepted.
> 
> Warn when the value does not follow the documented decimal format. Keep
> the existing parsing behavior for now so that users depending on these
> inputs are not broken; the warning can be promoted to an error later.
> The documented one-field value 50 gives tristate y/m/n probabilities of
> 25%/25%/50%. Leading whitespace and signs are accepted by strtol(), but are

This "25:25:50" explanation belongs in the first paragraph where you
give the background and state that "KCONFIG_PROBABILITY=50% is accepted
as 50:0:0". Your second paragraph is about what you're changing.

> also warned about because they are outside the documented format.
> Keep the strtol() result as long until the range check so that narrowing
> to int cannot turn an out-of-range value into a valid probability.
> 
> Add regression tests for malformed warnings and out-of-range values, the
> supported probability formats, and the documented empty-value default.
> 
> Fixes: e43956e60769 ("kconfig: implement KCONFIG_PROBABILITY for randconfig")
> Assisted-by: LLM
> Signed-off-by: Dmitrii Tulnov <tulnov.dl@xxxxxxxxx>
> ---
> Changes since v1:
> - Warn about malformed values while retaining their previous parsing behavior.
> - Treat leading whitespace and signs as warning-only undocumented input.
> - Clarify that 50 gives tristate y/m/n probabilities of 25%/25%/50%, and
>   compare it with the equivalent input 50:25:25 across 20 fixed seeds.
> 
> Validation on kbuild-next, x86_64, GCC 13.3.0:
> - make testconfig with HOSTCFLAGS=-Werror: 78 passed. Malformed values now
>   produce warnings while preserving the previous parsing behavior; numeric
>   range errors remain fatal.
> - ASan/UBSan at -O1, with leak detection disabled: the same 78 tests passed.
> - The fixed build accepted 916 of 1,635 generated inputs (including
>   warning-only malformed values) and rejected 719 numeric range errors.
>   It matched the original on 380 documented-input comparisons with fixed
>   seeds. Range errors preserved an existing .config, including with
>   KCONFIG_ALLCONFIG set.
> - make defconfig, allnoconfig, allmodconfig and valid randconfig passed.
>   KCONFIG_PROBABILITY=50% now produces a warning and remains compatible.
> - No vmlinux build or boot test; this changes the host configuration tool.
>   A 32-bit host build was unavailable because multilib headers were missing.
> 
>  scripts/kconfig/conf.c                        | 14 +++-
>  .../tests/randconfig_probability/Kconfig      | 12 +++
>  .../tests/randconfig_probability/__init__.py  | 79 +++++++++++++++++++
>  3 files changed, 104 insertions(+), 1 deletion(-)
>  create mode 100644 scripts/kconfig/tests/randconfig_probability/Kconfig
>  create mode 100644 scripts/kconfig/tests/randconfig_probability/__init__.py
> 
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index fe8ba09b0..33e8baf8d 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -186,12 +186,24 @@ static void conf_set_all_new_symbols(enum conf_def_mode mode)
>  
>  	if (mode == def_random) {
>  		int n, p[3];
> +		bool warned = false;
>  		char *env = getenv("KCONFIG_PROBABILITY");
>  
>  		n = 0;
>  		while (env && *env) {
>  			char *endp;
> -			int tmp = strtol(env, &endp, 10);
> +			long tmp = strtol(env, &endp, 10);
> +
> +			if (endp == env || isspace((unsigned char)*env) ||
> +			    *env == '+' || *env == '-' ||

This condition expression is pretty long. You can simplify this:

  endp == env || isspace((unsigned char)*env) ||
  *env == '+' || *env == '-'

into:

  !isdigit((unsigned char)*env)

> +			    (*endp && *endp != ':') ||
> +			    (*endp == ':' && (!endp[1] || n == 2))) {
> +				if (!warned) {
> +					fprintf(stderr,
> +						"warning: KCONFIG_PROBABILITY has malformed format\n");
> +					warned = true;
> +				}
> +			}
>  
>  			if (tmp >= 0 && tmp <= 100) {
>  				p[n++] = tmp;
> diff --git a/scripts/kconfig/tests/randconfig_probability/Kconfig b/scripts/kconfig/tests/randconfig_probability/Kconfig
> new file mode 100644
> index 000000000..84f4e5fcc
> --- /dev/null
> +++ b/scripts/kconfig/tests/randconfig_probability/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +config MODULES
> +	bool
> +	default y
> +	modules
> +
> +config BOOL
> +	bool "Bool"
> +
> +config TRI
> +	tristate "Tristate"
> diff --git a/scripts/kconfig/tests/randconfig_probability/__init__.py b/scripts/kconfig/tests/randconfig_probability/__init__.py
> new file mode 100644
> index 000000000..ff1770608
> --- /dev/null
> +++ b/scripts/kconfig/tests/randconfig_probability/__init__.py
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +"""Validate KCONFIG_PROBABILITY without changing the supported distributions."""
> +
> +import pytest
> +
> +
> +@pytest.mark.parametrize('probability', [
> +    'invalid', ' ', '50%', '50 ', '0x32', '10 20', '10:20x',
> +    '10:20:invalid', '10:20:30x',
> +    ':50', '50:', '10::20', '10:20:', '10:20:30:', '10:20:30:40',
> +    '+100:0', ' \t100:0', '0: \t100:0',
> +])
> +def test_malformed_warns(conf, monkeypatch, probability):
> +    monkeypatch.setenv('KCONFIG_PROBABILITY', probability)

You should set the environment variables using 'conf._run_conf' instead
of 'monkeypatch.setenv'.

For an example, see the test in:
scripts/kconfig/tests/warn_changed_input/
and how it uses 'extra_env'.

> +
> +    assert conf.randconfig(seed=0) == 0
> +    assert 'warning: KCONFIG_PROBABILITY has malformed format' in conf.stderr
> +    assert conf.config is not None
> +
> +
> +@pytest.mark.parametrize('probability', [
> +    '-1', '101', '0:101', '0:0:101', '60:41', '0:60:41',
> +    '4294967296', '-4294967296',
> +    '999999999999999999999999', '-999999999999999999999999',

It would be good to include '-0', which is the only negative input
accepted before this patch.

Also you should include an instance with '+'.

- Julian Braha