| Message ID | 1384711222-8916-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Accepted |
| Headers | show |
17.11.2013 22:00, Stefan Weil wrote: > gcc 4.8.2 reports this warning when extra warnings are enabled (-Wextra): > > CC m68k-softmmu/hw/m68k/mcf5206.o > hw/i386/acpi-build.c: In function ‘build_append_nameseg’: > hw/i386/acpi-build.c:294:5: error: > function might be possible candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format] > g_string_vprintf(s, format, args); > ^ Why are you sending for-1.7 patches which fixes issues which are not present in 1.7? As far as I can see, -Wextra isn't enabled in 1.7, is it? Thanks, /mjt
Am 17.11.2013 21:40, schrieb Michael Tokarev: > 17.11.2013 22:00, Stefan Weil wrote: >> gcc 4.8.2 reports this warning when extra warnings are enabled (-Wextra): >> >> CC m68k-softmmu/hw/m68k/mcf5206.o >> hw/i386/acpi-build.c: In function ‘build_append_nameseg’: >> hw/i386/acpi-build.c:294:5: error: >> function might be possible candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format] >> g_string_vprintf(s, format, args); >> ^ > Why are you sending for-1.7 patches which fixes issues which are not > present in 1.7? As far as I can see, -Wextra isn't enabled in 1.7, is it? > > Thanks, > > /mjt We try to use format attributes for all functions with printf like arguments. Patch http://patchwork.ozlabs.org/patch/291873/ shows that the format attribute not only fixes a compiler warning (which is not shown with the default settings) but also uncovers real programming errors. I use quite a lot of build environments (32 and 64 bit, Linux and Windows cross builds) but there still remain more which I don't cover. With the additional format attributes any code either is okay and compiles without warning, or it uses a wrong format string, so my patches will break only builds which compile broken code. I think this kind of build breakage is good and important. Regards, Stefan
17.11.2013 22:00, Stefan Weil wrote: > gcc 4.8.2 reports this warning when extra warnings are enabled (-Wextra): > > CC m68k-softmmu/hw/m68k/mcf5206.o > hw/i386/acpi-build.c: In function ‘build_append_nameseg’: > hw/i386/acpi-build.c:294:5: error: > function might be possible candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format] > g_string_vprintf(s, format, args); > ^ > > When this warning is fixed, there is a new compiler warning: > > CC i386-softmmu/hw/i386/acpi-build.o > hw/i386/acpi-build.c: In function ‘build_append_notify’: > hw/i386/acpi-build.c:632:5: error: > format not a string literal and no format arguments [-Werror=format-security] > build_append_nameseg(method, name); > ^ > > This is fixed here, too. Thanks, applied to the trivial-patches queue. Note this patch also triggers checkpatch.pl error: > +static void GCC_FMT_ATTR(2, 3) > +build_append_nameseg(GArray *array, const char *format, ...) ^ ERROR: need consistent spacing around '*' (ctx:WxV) #40: FILE: hw/i386/acpi-build.c:289: It looks like checkpatch.pl needs some tweaking here, because the spacing here is really consistent, as far as I can see. /mjt
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 486e705..9fff5ba 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -285,7 +285,8 @@ static inline void build_append_array(GArray *array, GArray *val) g_array_append_vals(array, val->data, val->len); } -static void build_append_nameseg(GArray *array, const char *format, ...) +static void GCC_FMT_ATTR(2, 3) +build_append_nameseg(GArray *array, const char *format, ...) { GString *s = g_string_new(""); va_list args; @@ -628,7 +629,7 @@ build_append_notify(GArray *device, const char *name, GArray *method = build_alloc_array(); uint8_t op = 0x14; /* MethodOp */ - build_append_nameseg(method, name); + build_append_nameseg(method, "%s", name); build_append_byte(method, 0x02); /* MethodFlags: ArgCount */ for (i = skip; i < count; i++) { GArray *target = build_alloc_array();
gcc 4.8.2 reports this warning when extra warnings are enabled (-Wextra): CC m68k-softmmu/hw/m68k/mcf5206.o hw/i386/acpi-build.c: In function ‘build_append_nameseg’: hw/i386/acpi-build.c:294:5: error: function might be possible candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format] g_string_vprintf(s, format, args); ^ When this warning is fixed, there is a new compiler warning: CC i386-softmmu/hw/i386/acpi-build.o hw/i386/acpi-build.c: In function ‘build_append_notify’: hw/i386/acpi-build.c:632:5: error: format not a string literal and no format arguments [-Werror=format-security] build_append_nameseg(method, name); ^ This is fixed here, too. Signed-off-by: Stefan Weil <sw@weilnetz.de> --- Note: checkpatch.pl wrongly reports an error. hw/i386/acpi-build.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)