Re: xentools411 fails build
This kind of warning (used with -Werror) breaks all sysutils/xentools*
builds with gcc >= 7.
I started fixing some for xentools48, but ran out of time and gave up
on xen instead.
Martin
$NetBSD$
Elide string truncation warning with newer gcc.
--- tools/xenpmd/xenpmd.c.orig 2018-01-23 14:49:58.0 +0100
+++ tools/xenpmd/xenpmd.c 2019-03-21 14:36:51.861782226 +0100
@@ -100,7 +100,7 @@ FILE *get_next_battery_file(DIR *battery
{
FILE *file = 0;
struct dirent *dir_entries;
-char file_name[32];
+char file_name[FILENAME_MAX];
do
{
@@ -110,10 +110,10 @@ FILE *get_next_battery_file(DIR *battery
if ( strlen(dir_entries->d_name) < 4 )
continue;
if ( battery_info_type == BIF )
-snprintf(file_name, 32, BATTERY_INFO_FILE_PATH,
+snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH,
dir_entries->d_name);
else
-snprintf(file_name, 32, BATTERY_STATE_FILE_PATH,
+snprintf(file_name, sizeof(file_name), BATTERY_STATE_FILE_PATH,
dir_entries->d_name);
file = fopen(file_name, "r");
} while ( !file );
$NetBSD$
Elide string truncation warning with newer gcc.
--- tools/misc/xenlockprof.c.orig 2018-01-23 14:49:58.0 +0100
+++ tools/misc/xenlockprof.c2019-03-21 14:29:57.595339322 +0100
@@ -24,7 +24,7 @@ int main(int argc, char *argv[])
uint32_t i, j, n;
uint64_t time;
double l, b, sl, sb;
-char name[60];
+char name[100];
DECLARE_HYPERCALL_BUFFER(xc_lockprof_data_t, data);
if ( (argc > 2) || ((argc == 2) && (strcmp(argv[1], "-r") != 0)) )
Re: xentools411 fails build
Date:Mon, 18 Mar 2019 11:30:49 +1100 From:matthew green Message-ID: <29602.1552869...@splode.eterna.com.au> | Martin Husemann writes: | > IMO the most stupid warning ever added to gcc. I would just disable it for | > newer gcc. | | i don't agree. some times it is very difficult to avoid and | it sometimes misdiagnoses the problem but it found a couple | of dozen real bugs where user input would lead to segv, so it | has clear value in my mind. I agree with Martin - and I fail to see how getting a truncated string (this is snprintf, no sprintf) could lead to a segv, ever (or if it did, the problem is elsewhere). It could lead to using the incorrect data (in this case presumanly attempting to open/create an unintended file) which might be bad, sometimes (just cause app failure in this case most likely - /tmp/battery/* is not any kind of critical path). But worse in this kind of usage, almost every occurrence of %s in a sprintf (that is, not %.Ns) would need to generate the warning, as there's no way that the compiler can know, for certain, that a variable string passed in will be properly \0 terminated within its apparent max size. It can guess sometimes, but it can never really know, unless it is a constant string. kre
Re: xentools411 fails build
On Mon, Mar 18, 2019 at 11:30:49AM +1100, matthew green wrote: > Martin Husemann writes: > > IMO the most stupid warning ever added to gcc. I would just disable it for > > newer gcc. > > i don't agree. some times it is very difficult to avoid and > it sometimes misdiagnoses the problem but it found a couple > of dozen real bugs where user input would lead to segv, so it > has clear value in my mind. OK, let me rephrase that: the idea of the warning is fine, but the implemenation makes it useless as there is no way you can avoid it in perfectly fine code, besides adding horrible hacks to the code that actually make the generated code worse. Martin
re: xentools411 fails build
Martin Husemann writes: > On Sat, Mar 16, 2019 at 10:18:48AM +, Chavdar Ivanov wrote: > > xenpmd.c: In function 'get_next_battery_file': > > xenpmd.c:90:36: error: '%s' directive output may be truncated writing > > up to 511 bytes into a region of size 271 [-Werror=format-truncation=] > > #define BATTERY_INFO_FILE_PATH "/tmp/battery/%s/info" > > ^ > > xenpmd.c:113:52: note: in expansion of macro 'BATTERY_INFO_FILE_PATH' > > snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH, > > ^~ > > xenpmd.c:113:13: note: 'snprintf' output between 19 and 530 bytes into > > a destination of size 284 > > snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH, > > ^~ > > dir_entries->d_name); > > > > IMO the most stupid warning ever added to gcc. I would just disable it for > newer gcc. i don't agree. some times it is very difficult to avoid and it sometimes misdiagnoses the problem but it found a couple of dozen real bugs where user input would lead to segv, so it has clear value in my mind. .mrg.
Re: xentools411 fails build
xentools411 builds fine now. Next I might try it with the new ocaml from wip. On Sat, 16 Mar 2019 at 16:39, Manuel Bouyer wrote: > > On Sat, Mar 16, 2019 at 04:43:43PM +0200, Andreas Gustafsson wrote: > > Martin Husemann wrote: > > > IMO the most stupid warning ever added to gcc. I would just disable it for > > > newer gcc. > > > > Be that as it may, gutteridge@ already fixed the xentools411 issue by > > adding xentools411/patches/patch-tools_xenpmd_xenpmd.c on Feb 25. > > Perhaps Chavdar's problem has something to do with bouyer@ removing > > that patch on March 7? > > I re-added it, sorry for the breakage > > -- > Manuel Bouyer > NetBSD: 26 ans d'experience feront toujours la difference > -- --
Re: xentools411 fails build
On Sat, Mar 16, 2019 at 04:43:43PM +0200, Andreas Gustafsson wrote: > Martin Husemann wrote: > > IMO the most stupid warning ever added to gcc. I would just disable it for > > newer gcc. > > Be that as it may, gutteridge@ already fixed the xentools411 issue by > adding xentools411/patches/patch-tools_xenpmd_xenpmd.c on Feb 25. > Perhaps Chavdar's problem has something to do with bouyer@ removing > that patch on March 7? I re-added it, sorry for the breakage -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: xentools411 fails build
Martin Husemann wrote: > IMO the most stupid warning ever added to gcc. I would just disable it for > newer gcc. Be that as it may, gutteridge@ already fixed the xentools411 issue by adding xentools411/patches/patch-tools_xenpmd_xenpmd.c on Feb 25. Perhaps Chavdar's problem has something to do with bouyer@ removing that patch on March 7? -- Andreas Gustafsson, g...@gson.org
Re: xentools411 fails build
On Sat, Mar 16, 2019 at 10:18:48AM +, Chavdar Ivanov wrote: > xenpmd.c: In function 'get_next_battery_file': > xenpmd.c:90:36: error: '%s' directive output may be truncated writing > up to 511 bytes into a region of size 271 [-Werror=format-truncation=] > #define BATTERY_INFO_FILE_PATH "/tmp/battery/%s/info" > ^ > xenpmd.c:113:52: note: in expansion of macro 'BATTERY_INFO_FILE_PATH' > snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH, > ^~ > xenpmd.c:113:13: note: 'snprintf' output between 19 and 530 bytes into > a destination of size 284 > snprintf(file_name, sizeof(file_name), BATTERY_INFO_FILE_PATH, > ^~ > dir_entries->d_name); > IMO the most stupid warning ever added to gcc. I would just disable it for newer gcc. Martin
