* [PATCH] tools: fix several "format-truncation" errors with GCC 7
@ 2017-06-12 12:07 Zhongze Liu
2017-06-12 12:31 ` Ian Jackson
0 siblings, 1 reply; 3+ messages in thread
From: Zhongze Liu @ 2017-06-12 12:07 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Ian Jackson, Zhongze Liu, David Scott
replace several snprintf with asprintf in xenpmd and tools/ocmal/xc
to fix the "format-truncation" errors caused by incorrect size of buffers.
Signed-off-by: Zhongze Liu <blackskygg@gmail.com>
---
CC: David Scott <dave@recoil.org>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/ocaml/libs/xc/xenctrl_stubs.c | 12 +++++-------
tools/xenpmd/xenpmd.c | 18 ++++++++++++------
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 5e455519d4..7011930360 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -14,6 +14,7 @@
* GNU Lesser General Public License for more details.
*/
+#define _GNU_SOURCE
#define _XOPEN_SOURCE 600
#include <stdlib.h>
#include <errno.h>
@@ -54,20 +55,17 @@
static void Noreturn failwith_xc(xc_interface *xch)
{
- char error_str[256];
+ char *error_str = NULL;
if (xch) {
const xc_error *error = xc_get_last_error(xch);
if (error->code == XC_ERROR_NONE)
- snprintf(error_str, sizeof(error_str),
- "%d: %s", errno, strerror(errno));
+ asprintf(&error_str, "%d: %s", errno, strerror(errno));
else
- snprintf(error_str, sizeof(error_str),
- "%d: %s: %s", error->code,
+ asprintf(&error_str, "%d: %s: %s", error->code,
xc_error_code_to_desc(error->code),
error->message);
} else {
- snprintf(error_str, sizeof(error_str),
- "Unable to open XC interface");
+ asprintf(&error_str, "Unable to open XC interface");
}
caml_raise_with_string(*caml_named_value("xc.error"), error_str);
}
diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
index b3a31062aa..2f4ef45a11 100644
--- a/tools/xenpmd/xenpmd.c
+++ b/tools/xenpmd/xenpmd.c
@@ -32,6 +32,7 @@
* passed to the guest when appropriate battery ports are read/written to.
*/
+#define _GNU_SOURCE
#include <stdio.h>
#include <stdarg.h>
#include <string.h>
@@ -100,7 +101,8 @@ FILE *get_next_battery_file(DIR *battery_dir,
{
FILE *file = 0;
struct dirent *dir_entries;
- char file_name[32];
+ char *file_name = NULL;
+ int rc = 0;
do
{
@@ -110,12 +112,16 @@ FILE *get_next_battery_file(DIR *battery_dir,
if ( strlen(dir_entries->d_name) < 4 )
continue;
if ( battery_info_type == BIF )
- snprintf(file_name, 32, BATTERY_INFO_FILE_PATH,
- dir_entries->d_name);
+ rc = asprintf(&file_name, BATTERY_INFO_FILE_PATH,
+ dir_entries->d_name);
else
- snprintf(file_name, 32, BATTERY_STATE_FILE_PATH,
- dir_entries->d_name);
- file = fopen(file_name, "r");
+ rc = asprintf(&file_name, BATTERY_STATE_FILE_PATH,
+ dir_entries->d_name);
+
+ if ( -1 != rc ) {
+ file = fopen(file_name, "r");
+ free(file_name);
+ }
} while ( !file );
return file;
--
2.13.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tools: fix several "format-truncation" errors with GCC 7
2017-06-12 12:07 [PATCH] tools: fix several "format-truncation" errors with GCC 7 Zhongze Liu
@ 2017-06-12 12:31 ` Ian Jackson
2017-06-12 17:55 ` Zhongze Liu
0 siblings, 1 reply; 3+ messages in thread
From: Ian Jackson @ 2017-06-12 12:31 UTC (permalink / raw)
To: Zhongze Liu; +Cc: xen-devel, Wei Liu, David Scott
Zhongze Liu writes ("[PATCH] tools: fix several "format-truncation" errors with GCC 7"):
> replace several snprintf with asprintf in xenpmd and tools/ocmal/xc
> to fix the "format-truncation" errors caused by incorrect size of buffers.
Thanks for paying attention to the quality of our code, but:
I wonder whether this cure is worse than the disease. Using asprintf
everywhere means additional error handling (which you have erroneously
omitted) and additional potential for leaks etc. (for which I haven't
analysed your patch).
You say `"format-truncation" errors' but you mean compiler warnings
from -Wformat-truncation, turned into errors by -Werror. Is there
any suggestion from a human that this code actually malfunctions ?
Or does the compiler not just complain all the time about snprintf ?
> - char error_str[256];
...
> - snprintf(error_str, sizeof(error_str),
> - "%d: %s", errno, strerror(errno));
This will not truncate unless the xc error string is too long, which
is not.
> - snprintf(error_str, sizeof(error_str),
> - "Unable to open XC interface");
> + asprintf(&error_str, "Unable to open XC interface");
This is a fixed string of course.
> - char file_name[32];
...
> @@ -110,12 +112,16 @@ FILE *get_next_battery_file(DIR *battery_dir,
> if ( strlen(dir_entries->d_name) < 4 )
> continue;
> if ( battery_info_type == BIF )
> - snprintf(file_name, 32, BATTERY_INFO_FILE_PATH,
> - dir_entries->d_name);
> + rc = asprintf(&file_name, BATTERY_INFO_FILE_PATH,
> + dir_entries->d_name);
> else
> - snprintf(file_name, 32, BATTERY_STATE_FILE_PATH,
> - dir_entries->d_name);
> - file = fopen(file_name, "r");
> + rc = asprintf(&file_name, BATTERY_STATE_FILE_PATH,
> + dir_entries->d_name);
These filenames are all very formulaic. I doubt they are being
truncated even though the limit is only 32 bytes.
Regards,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tools: fix several "format-truncation" errors with GCC 7
2017-06-12 12:31 ` Ian Jackson
@ 2017-06-12 17:55 ` Zhongze Liu
0 siblings, 0 replies; 3+ messages in thread
From: Zhongze Liu @ 2017-06-12 17:55 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Wei Liu, David Scott
Hi Ian,
Thanks for pointing out the problems. I've consulted several
maintainers about this
and have drafted a new patch for it (in a new [patch v2] thread).
Please have a look
at it. Thanks.
Cheers,
Zhongze Liu.
2017-06-12 20:31 GMT+08:00 Ian Jackson <ian.jackson@eu.citrix.com>:
> Zhongze Liu writes ("[PATCH] tools: fix several "format-truncation" errors with GCC 7"):
>> replace several snprintf with asprintf in xenpmd and tools/ocmal/xc
>> to fix the "format-truncation" errors caused by incorrect size of buffers.
>
> Thanks for paying attention to the quality of our code, but:
>
> I wonder whether this cure is worse than the disease. Using asprintf
> everywhere means additional error handling (which you have erroneously
> omitted) and additional potential for leaks etc. (for which I haven't
> analysed your patch).
>
> You say `"format-truncation" errors' but you mean compiler warnings
> from -Wformat-truncation, turned into errors by -Werror. Is there
> any suggestion from a human that this code actually malfunctions ?
>
> Or does the compiler not just complain all the time about snprintf ?
>
>> - char error_str[256];
> ...
>> - snprintf(error_str, sizeof(error_str),
>> - "%d: %s", errno, strerror(errno));
>
> This will not truncate unless the xc error string is too long, which
> is not.
>
>> - snprintf(error_str, sizeof(error_str),
>> - "Unable to open XC interface");
>> + asprintf(&error_str, "Unable to open XC interface");
>
> This is a fixed string of course.
>
>> - char file_name[32];
> ...
>> @@ -110,12 +112,16 @@ FILE *get_next_battery_file(DIR *battery_dir,
>> if ( strlen(dir_entries->d_name) < 4 )
>> continue;
>> if ( battery_info_type == BIF )
>> - snprintf(file_name, 32, BATTERY_INFO_FILE_PATH,
>> - dir_entries->d_name);
>> + rc = asprintf(&file_name, BATTERY_INFO_FILE_PATH,
>> + dir_entries->d_name);
>> else
>> - snprintf(file_name, 32, BATTERY_STATE_FILE_PATH,
>> - dir_entries->d_name);
>> - file = fopen(file_name, "r");
>> + rc = asprintf(&file_name, BATTERY_STATE_FILE_PATH,
>> + dir_entries->d_name);
>
> These filenames are all very formulaic. I doubt they are being
> truncated even though the limit is only 32 bytes.
>
> Regards,
> Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-12 17:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-12 12:07 [PATCH] tools: fix several "format-truncation" errors with GCC 7 Zhongze Liu
2017-06-12 12:31 ` Ian Jackson
2017-06-12 17:55 ` Zhongze Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.