public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cpupower: replace atoi() with validated parse_int_range() helper
@ 2026-03-31  4:28 Kaushlendra Kumar
  2026-03-31 23:53 ` Shuah Khan
  0 siblings, 1 reply; 3+ messages in thread
From: Kaushlendra Kumar @ 2026-03-31  4:28 UTC (permalink / raw)
  To: shuah, jwyatt, jkacur; +Cc: linux-pm, Kaushlendra Kumar

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.

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;
+}
+
 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);
+			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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpupower: replace atoi() with validated parse_int_range() helper
  2026-03-31  4:28 [PATCH] cpupower: replace atoi() with validated parse_int_range() helper Kaushlendra Kumar
@ 2026-03-31 23:53 ` Shuah Khan
  2026-04-01  2:57   ` Kumar, Kaushlendra
  0 siblings, 1 reply; 3+ messages in thread
From: Shuah Khan @ 2026-03-31 23:53 UTC (permalink / raw)
  To: Kaushlendra Kumar, shuah, jwyatt, jkacur; +Cc: linux-pm, Shuah Khan

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] cpupower: replace atoi() with validated parse_int_range() helper
  2026-03-31 23:53 ` Shuah Khan
@ 2026-04-01  2:57   ` Kumar, Kaushlendra
  0 siblings, 0 replies; 3+ messages in thread
From: Kumar, Kaushlendra @ 2026-04-01  2:57 UTC (permalink / raw)
  To: Shuah Khan, shuah@kernel.org, jwyatt@redhat.com,
	jkacur@redhat.com
  Cc: linux-pm@vger.kernel.org

On 3/30/26 22:28, Shuah Khan wrote:
> Adding a routine like this is not the right direction because it adds
> code that needs to be maintained.

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

Agreed.

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

No, there is no problem with the existing range check logic. 
We will revert to the original approach.

Thanks,
Kaushlendra Kumar

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-01  2:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31  4:28 [PATCH] cpupower: replace atoi() with validated parse_int_range() helper Kaushlendra Kumar
2026-03-31 23:53 ` Shuah Khan
2026-04-01  2:57   ` Kumar, Kaushlendra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox