[PATCH] power: fix pstate number parsing

[PATCH] power: fix pstate number parsing

[Markus Theil]

From: Markus Theil <markus.theil@secunet.com>

When converting atoi to strtol in a revision
of introducing sysfs support for turbo percentage,
a necessary check against '\n' returned by sysfs
was not introduced.

Fixes: de254dac608e ("power: read P-state turbo percentage from sysfs")
Signed-off-by: Markus Theil <markus.theil@secunet.com>
---
 lib/power/power_pstate_cpufreq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/power/power_pstate_cpufreq.c b/lib/power/power_pstate_cpufreq.c
index 49ddb2eefd..7e660618db 100644
--- a/lib/power/power_pstate_cpufreq.c
+++ b/lib/power/power_pstate_cpufreq.c
@@ -2,6 +2,7 @@
  * Copyright(c) 2018 Intel Corporation
  */
 
+#include <ctype.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <fcntl.h>
@@ -96,7 +97,7 @@ power_read_turbo_pct(uint64_t *outVal)
 
 	errno = 0;
 	*outVal = (uint64_t) strtol(val, &endptr, 10);
-	if (*endptr != 0 || errno != 0) {
+	if (errno != 0 || (*endptr != 0 && *endptr != '\n')) {
 		RTE_LOG(ERR, POWER, "Error converting str to digits, read from %s: %s\n",
 				 POWER_SYSFILE_TURBO_PCT, strerror(errno));
 		ret = -1;
-- 
2.38.0

RE: [PATCH] power: fix pstate number parsing

[Pattan, Reshma]

> -----Original Message-----
> From: Markus Theil <markus.theil@tu-ilmenau.de>


> +#include <ctype.h>

This is not needed right.

>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <fcntl.h>
> @@ -96,7 +97,7 @@ power_read_turbo_pct(uint64_t *outVal)
> 
>  	errno = 0;
>  	*outVal = (uint64_t) strtol(val, &endptr, 10);
> -	if (*endptr != 0 || errno != 0) {
> +	if (errno != 0 || (*endptr != 0 && *endptr != '\n')) {

I encountered today that power library initialization failed and the reason is this \n check. 
This fix fixes the issue.

So if you are sending the next version be removing the above mentioned header file, please keep my Review and Ack tags in next version.

Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

[PATCH v2] power: fix pstate number parsing

[Markus Theil]

From: Markus Theil <markus.theil@secunet.com>

When converting atoi to strtol in a revision
of introducing sysfs support for turbo percentage,
a necessary check against '\n' returned by sysfs
was not introduced.

Fixes: de254dac608e ("power: read P-state turbo percentage from sysfs")
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>
Signed-off-by: Markus Theil <markus.theil@secunet.com>
---
 lib/power/power_pstate_cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/power/power_pstate_cpufreq.c b/lib/power/power_pstate_cpufreq.c
index 49ddb2eefd..52aa64510e 100644
--- a/lib/power/power_pstate_cpufreq.c
+++ b/lib/power/power_pstate_cpufreq.c
@@ -96,7 +96,7 @@ power_read_turbo_pct(uint64_t *outVal)

 	errno = 0;
 	*outVal = (uint64_t) strtol(val, &endptr, 10);
-	if (*endptr != 0 || errno != 0) {
+	if (errno != 0 || (*endptr != 0 && *endptr != '\n')) {
 		RTE_LOG(ERR, POWER, "Error converting str to digits, read from %s: %s\n",
 				 POWER_SYSFILE_TURBO_PCT, strerror(errno));
 		ret = -1;
--
2.38.0

Re: [PATCH] power: fix pstate number parsing

[Thomas Monjalon]

12/10/2022 14:25, Pattan, Reshma:
> 
> > -----Original Message-----
> > From: Markus Theil <markus.theil@tu-ilmenau.de>
> 
> 
> > +#include <ctype.h>
> 
> This is not needed right.
> 
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <fcntl.h>
> > @@ -96,7 +97,7 @@ power_read_turbo_pct(uint64_t *outVal)
> > 
> >  	errno = 0;
> >  	*outVal = (uint64_t) strtol(val, &endptr, 10);
> > -	if (*endptr != 0 || errno != 0) {
> > +	if (errno != 0 || (*endptr != 0 && *endptr != '\n')) {
> 
> I encountered today that power library initialization failed and the reason is this \n check. 
> This fix fixes the issue.
> 
> So if you are sending the next version be removing the above mentioned header file, please keep my Review and Ack tags in next version.

Review is enough, it is stronger than ack.
Review means you carefully reviewed the change.

Re: [PATCH v2] power: fix pstate number parsing

[Thomas Monjalon]

12/10/2022 14:36, Markus Theil:
> From: Markus Theil <markus.theil@secunet.com>
> 
> When converting atoi to strtol in a revision
> of introducing sysfs support for turbo percentage,
> a necessary check against '\n' returned by sysfs
> was not introduced.
> 
> Fixes: de254dac608e ("power: read P-state turbo percentage from sysfs")
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
> Acked-by: Reshma Pattan <reshma.pattan@intel.com>
> Signed-off-by: Markus Theil <markus.theil@secunet.com>

Applied, thanks.