[PATCH v2] xenpmd.c: use dynamic allocation

Manuel Bouyer posted 1 patch 3 years, 3 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210126224800.1246-14-bouyer@netbsd.org
tools/xenpmd/xenpmd.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH v2] xenpmd.c: use dynamic allocation
Posted by Manuel Bouyer 3 years, 3 months ago
On NetBSD, d_name is larger than 256, so file_name[284] may not be large
enough (and gcc emits a format-truncation error).
Use asprintf() instead of snprintf() on a static on-stack buffer.

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
---
 tools/xenpmd/xenpmd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
index 12b82cf43e..e432aad856 100644
--- a/tools/xenpmd/xenpmd.c
+++ b/tools/xenpmd/xenpmd.c
@@ -101,7 +101,7 @@ FILE *get_next_battery_file(DIR *battery_dir,
 {
     FILE *file = 0;
     struct dirent *dir_entries;
-    char file_name[284];
+    char *file_name;
     int ret;
     
     do 
@@ -112,16 +112,16 @@ FILE *get_next_battery_file(DIR *battery_dir,
         if ( strlen(dir_entries->d_name) < 4 )
             continue;
         if ( battery_info_type == BIF ) 
-            ret = snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH,
+            ret = asprintf(&file_name, BATTERY_INFO_FILE_PATH,
                      dir_entries->d_name);
         else 
-            ret = snprintf(file_name, sizeof(file_name), BATTERY_STATE_FILE_PATH,
+            ret = asprintf(&file_name, BATTERY_STATE_FILE_PATH,
                      dir_entries->d_name);
         /* This should not happen but is needed to pass gcc checks */
         if (ret < 0)
             continue;
-        file_name[sizeof(file_name) - 1] = '\0';
         file = fopen(file_name, "r");
+	free(file_name);
     } while ( !file );
 
     return file;
-- 
2.29.2


Re: [PATCH v2] xenpmd.c: use dynamic allocation
Posted by Roger Pau Monné 3 years, 2 months ago
On Tue, Jan 26, 2021 at 11:48:00PM +0100, Manuel Bouyer wrote:
> On NetBSD, d_name is larger than 256, so file_name[284] may not be large
> enough (and gcc emits a format-truncation error).
> Use asprintf() instead of snprintf() on a static on-stack buffer.
> 
> Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
>  tools/xenpmd/xenpmd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
> index 12b82cf43e..e432aad856 100644
> --- a/tools/xenpmd/xenpmd.c
> +++ b/tools/xenpmd/xenpmd.c
> @@ -101,7 +101,7 @@ FILE *get_next_battery_file(DIR *battery_dir,
>  {
>      FILE *file = 0;
>      struct dirent *dir_entries;
> -    char file_name[284];
> +    char *file_name;
>      int ret;
>      
>      do 
> @@ -112,16 +112,16 @@ FILE *get_next_battery_file(DIR *battery_dir,
>          if ( strlen(dir_entries->d_name) < 4 )
>              continue;
>          if ( battery_info_type == BIF ) 
> -            ret = snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH,
> +            ret = asprintf(&file_name, BATTERY_INFO_FILE_PATH,
>                       dir_entries->d_name);
>          else 
> -            ret = snprintf(file_name, sizeof(file_name), BATTERY_STATE_FILE_PATH,
> +            ret = asprintf(&file_name, BATTERY_STATE_FILE_PATH,
>                       dir_entries->d_name);
>          /* This should not happen but is needed to pass gcc checks */
>          if (ret < 0)
>              continue;
> -        file_name[sizeof(file_name) - 1] = '\0';
>          file = fopen(file_name, "r");
> +	free(file_name);

Hard tab. May I ask whether this can be fixed on commit so that
there's no need to resend a new version?

Thanks, Roger.