diff mbox series

power: fix pstate number parsing

Message ID 20221012113342.7931-1-markus.theil@tu-ilmenau.de (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series power: fix pstate number parsing |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK
ci/intel-Testing success Testing PASS

Commit Message

Markus Theil Oct. 12, 2022, 11:33 a.m. UTC 
  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(-)

Comments

Pattan, Reshma Oct. 12, 2022, 12:25 p.m. UTC | #1 
> -----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>
Thomas Monjalon Oct. 26, 2022, 9:33 p.m. UTC | #2 
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.
diff mbox series

Patch

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;