Re: [Powertop] [PATCH v2 5/8] Limit sprintf to buffer size length.

Re: [Powertop] [PATCH v2 5/8] Limit sprintf to buffer size length.

[Sergey Senozhatsky]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On (10/14/14 11:09), Joe Konno wrote:
> Not completely needed, but good practice.  Limiting sprintf to the
> length of the buffer size.  This patch not completely necessary as it's
> unlikely to be an issue here.
> 
> v2: rebase atop 41c54e8; adaptated to newer context
> 
> Signed-off-by: Joe Konno <joe.konno(a)intel.com>
> ---
>  src/lib.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lib.cpp b/src/lib.cpp
> index 437803b..9cbf78d 100644
> --- a/src/lib.cpp
> +++ b/src/lib.cpp
> @@ -474,10 +474,10 @@ int read_msr(int cpu, uint64_t offset, uint64_t *value)
>  	int fd;
>  	char msr_path[256];
>  
> -	sprintf(msr_path, "/dev/cpu/%d/msr", cpu);
> +	fd = snprintf(msr_path, 256, "/dev/cpu/%d/msr", cpu);

iirc, this `fd =' has been removed. it's useless per se.

	-ss

>  	if (access(msr_path, R_OK) != 0){
> -		sprintf(msr_path, "/dev/msr%d", cpu);
> +		fd = snprintf(msr_path, 256, "/dev/msr%d", cpu);
>  
>  		if (access(msr_path, R_OK) != 0){
>  			fprintf(stderr,
> @@ -506,10 +506,10 @@ int write_msr(int cpu, uint64_t offset, uint64_t value)
>  	int fd;
>  	char msr_path[256];
>  
> -	sprintf(msr_path, "/dev/cpu/%d/msr", cpu);
> +	fd = snprintf(msr_path, 256, "/dev/cpu/%d/msr", cpu);
>  
>  	if (access(msr_path, R_OK) != 0){
> -		sprintf(msr_path, "/dev/msr%d", cpu);
> +		fd = snprintf(msr_path, 256, "/dev/msr%d", cpu);
>  
>  		if (access(msr_path, R_OK) != 0){
>  			fprintf(stderr,
> -- 
> 2.1.2
> 
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
> 

Re: [Powertop] [PATCH v2 5/8] Limit sprintf to buffer size length.

[Joe Konno]

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]

On 10/15/2014 05:15 AM, Sergey Senozhatsky wrote:
> On (10/14/14 11:09), Joe Konno wrote:
>> diff --git a/src/lib.cpp b/src/lib.cpp
>> index 437803b..9cbf78d 100644
>> --- a/src/lib.cpp
>> +++ b/src/lib.cpp
>> @@ -474,10 +474,10 @@ int read_msr(int cpu, uint64_t offset, uint64_t *value)
>>  	int fd;
>>  	char msr_path[256];
>>  
>> -	sprintf(msr_path, "/dev/cpu/%d/msr", cpu);
>> +	fd = snprintf(msr_path, 256, "/dev/cpu/%d/msr", cpu);
> 
> iirc, this `fd =' has been removed. it's useless per se.
> 
> 	-ss

Will fix.

[Powertop] [PATCH v2 5/8] Limit sprintf to buffer size length.

[Joe Konno]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

From: Dan Kalowsky <daniel.kalowsky(a)intel.com>

Not completely needed, but good practice.  Limiting sprintf to the
length of the buffer size.  This patch not completely necessary as it's
unlikely to be an issue here.

v2: rebase atop 41c54e8; adaptated to newer context

Signed-off-by: Joe Konno <joe.konno(a)intel.com>
---
 src/lib.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/lib.cpp b/src/lib.cpp
index 437803b..9cbf78d 100644
--- a/src/lib.cpp
+++ b/src/lib.cpp
@@ -474,10 +474,10 @@ int read_msr(int cpu, uint64_t offset, uint64_t *value)
 	int fd;
 	char msr_path[256];
 
-	sprintf(msr_path, "/dev/cpu/%d/msr", cpu);
+	fd = snprintf(msr_path, 256, "/dev/cpu/%d/msr", cpu);
 
 	if (access(msr_path, R_OK) != 0){
-		sprintf(msr_path, "/dev/msr%d", cpu);
+		fd = snprintf(msr_path, 256, "/dev/msr%d", cpu);
 
 		if (access(msr_path, R_OK) != 0){
 			fprintf(stderr,
@@ -506,10 +506,10 @@ int write_msr(int cpu, uint64_t offset, uint64_t value)
 	int fd;
 	char msr_path[256];
 
-	sprintf(msr_path, "/dev/cpu/%d/msr", cpu);
+	fd = snprintf(msr_path, 256, "/dev/cpu/%d/msr", cpu);
 
 	if (access(msr_path, R_OK) != 0){
-		sprintf(msr_path, "/dev/msr%d", cpu);
+		fd = snprintf(msr_path, 256, "/dev/msr%d", cpu);
 
 		if (access(msr_path, R_OK) != 0){
 			fprintf(stderr,
-- 
2.1.2