Re: [PATCH] cpupower: replace atoi() with validated parse_int_range() helper

[Shuah Khan <skhan@linuxfoundation.org>]

On 3/30/26 22:28, Kaushlendra Kumar wrote:
> atoi() provides no error detection for invalid input or out-of-range
> values. Introduce parse_int_range(), which uses strtol() with full
> errno and end-pointer checking, returning -EINVAL on parse failure
> and -ERANGE when the value falls outside the caller-supplied [min,
> max] bounds.
> 
> Switch --perf-bias and --turbo-boost option parsing to use this
> helper. Also move error messages from printf() to fprintf(stderr)
> to follow standard CLI conventions for diagnostic output.

What errors are you seeing?
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>   tools/power/cpupower/utils/cpupower-set.c | 29 +++++++++++++++++------
>   1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpupower-set.c b/tools/power/cpupower/utils/cpupower-set.c
> index 550a942e72ce..dcbdff5c9ae5 100644
> --- a/tools/power/cpupower/utils/cpupower-set.c
> +++ b/tools/power/cpupower/utils/cpupower-set.c
> @@ -31,6 +31,24 @@ static void print_wrong_arg_exit(void)
>   	exit(EXIT_FAILURE);
>   }
>   
> +/* Safe integer parsing with range validation. Returns 0 on success,
> + * -EINVAL on parse error, -ERANGE if value is outside [min, max].
> + */
> +static int parse_int_range(const char *arg, int min, int max, int *out)
> +{
> +	char *end = NULL;
> +	long val;
> +
> +	errno = 0;
> +	val = strtol(arg, &end, 10);
> +	if (errno || end == arg || *end != '\0')
> +		return -EINVAL;
> +	if (val < min || val > max)
> +		return -ERANGE;
> +	*out = (int)val;
> +	return 0;
> +}

Adding a routine like this is not the right direction
because it adds code that needs to be maintained.

> +
>   int cmd_set(int argc, char **argv)
>   {
>   	extern char *optarg;
> @@ -69,10 +87,8 @@ int cmd_set(int argc, char **argv)
>   		case 'b':
>   			if (params.perf_bias)
>   				print_wrong_arg_exit();
> -			perf_bias = atoi(optarg);
> -			if (perf_bias < 0 || perf_bias > 15) {
> -				printf(_("--perf-bias param out "
> -					 "of range [0-%d]\n"), 15);

The above logic range checks on perf_bias - Is there a problem
in this logic?


> +			if (parse_int_range(optarg, 0, 15, &perf_bias)) {
> +				fprintf(stderr, _("--perf-bias param out of range [0-%d]\n"), 15);
>   				print_wrong_arg_exit();
>   			}
>   			params.perf_bias = 1;
> @@ -100,9 +116,8 @@ int cmd_set(int argc, char **argv)
>   		case 't':
>   			if (params.turbo_boost)
>   				print_wrong_arg_exit();
> -			turbo_boost = atoi(optarg);
> -			if (turbo_boost < 0 || turbo_boost > 1) {
> -				printf("--turbo-boost param out of range [0-1]\n");
> +			if (parse_int_range(optarg, 0, 1, &turbo_boost)) {
> +				fprintf(stderr, "--turbo-boost param out of range [0-1]\n");
>   				print_wrong_arg_exit();
>   			}
>   			params.turbo_boost = 1;

I am not convinced there is need for a new routine to do the checks.

thanks,
-- Shuah