All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Hancock <hancockrwd@gmail.com>
To: Roland Dreier <rdreier@cisco.com>
Cc: Floris Kraak <randakar@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Trivial Patch Monkey <trivial@kernel.org>,
	Andreas Schwab <schwab@suse.de>
Subject: Re: [PATCH]: Cleanup: Remove gcc format string warnings when compiling with -Wformat-security
Date: Thu, 05 Feb 2009 17:42:52 -0600	[thread overview]
Message-ID: <498B797C.4060202@gmail.com> (raw)
In-Reply-To: <adak584jzw3.fsf@cisco.com>

Roland Dreier wrote:
>  > Here's the patch that I get when I blindly patch every single location
>  > that emits this warning.
> 
> 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

"const char* const" text would likely work as well..

> 
> 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);

Well, in that case, printk("foo") would be the obvious solution :-)

> 
> Would be nice to think of a cleverer way to handle that...
> 
>  - R.


  parent reply	other threads:[~2009-02-05 23:43 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-05 14:41 [PATCH]: Cleanup: Remove gcc format string warnings when compiling with -Wformat-security (was: Re: [PATCH] Kbuild: Disable the -Wformat-security gcc flag) Floris Kraak
2009-02-05 14:52 ` Ingo Molnar
2009-02-05 15:22 ` Mike Isely
2009-02-05 15:38   ` Floris Kraak
2009-02-05 16:34 ` Floris Kraak
2009-02-05 22:13 ` [PATCH]: Cleanup: Remove gcc format string warnings when compiling with -Wformat-security Roland Dreier
2009-02-05 22:41   ` Floris Kraak
2009-02-05 23:42   ` Robert Hancock [this message]
2009-02-06 10:36     ` Floris Kraak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=498B797C.4060202@gmail.com \
    --to=hancockrwd@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=randakar@gmail.com \
    --cc=rdreier@cisco.com \
    --cc=sam@ravnborg.org \
    --cc=schwab@suse.de \
    --cc=trivial@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.