Messages in this thread |  | | | Date | Thu, 5 Feb 2009 23:41:34 +0100 | | Subject | Re: [PATCH]: Cleanup: Remove gcc format string warnings when compiling with -Wformat-security | | From | Floris Kraak <> |
| |
On Thu, Feb 5, 2009 at 11:13 PM, Roland Dreier <rdreier@cisco.com> wrote: > > I would strongly prefer to do this with a little more care. For example > the b43/main.c change: > > > --- a/drivers/net/wireless/b43/main.c > > +++ b/drivers/net/wireless/b43/main.c > > @@ -2005,9 +2005,9 @@ static void b43_print_fw_helptext(struct b43_wl > > *wl, bool error) > > "http://linuxwireless.org/en/users/Drivers/b43#devicefirmware " > > "and download the latest firmware (version 4).\n"; > > if (error) > > - b43err(wl, text); > > + b43err(wl, "%s", text); > > else > > - b43warn(wl, text); > > + b43warn(wl, "%s", text); > > } > would probably be better solved by doing > > --- a/drivers/net/wireless/b43/main.c > +++ b/drivers/net/wireless/b43/main.c > @@ -1999,11 +1999,11 @@ static void b43_release_firmware(struct b43_wldev *dev) > > static void b43_print_fw_helptext(struct b43_wl *wl, bool error) > { > - const char *text; > + static const char text[] = > + "You must go to " > + "http://linuxwireless.org/en/users/Drivers/b43#devicefirmware " > + "and download the latest firmware (version 4).\n"; > > - text = "You must go to " > - "http://linuxwireless.org/en/users/Drivers/b43#devicefirmware " > - "and download the latest firmware (version 4).\n"; > if (error) > b43err(wl, text); > else >
Fair enough. I'm going to have to break this down into categories of bugs probably anyway. - "trivial just makes the compiler stops whining" vs "omg security hole" and the shades in between. From what I've seen so far the vast majority of this falls into the first category though ;-)
> and in any case I'm not totally convinced that we want to add the bloat > for trivial cases like > > char *safe = "foo"; > printk(safe); > > Would be nice to think of a cleverer way to handle that... >
It would help if GCC was a little smarter in the detection, right now it gives a lot of false positives even on code like that. When I was playing around with coccinelle tonight in a (probably misguided) attempt to automate this I found a few new possible cases where macro wrappers around printk() seem to make avoiding this check possible. Haven't looked deeply yet, I'm doing this on my spare time which I don't have tons of ;-)
Regards, Floris --- "They that give up essential liberty to obtain temporary safety, deserve neither liberty nor safety." -- Ben Franklin "The course of history shows that as a government grows, liberty decreases." -- Thomas Jefferson
|  |