Re: [patch] fix warnings with -D_FORTIFY_SOURCE and -Wformat-security

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Sun, 15 Mar 2009, Matthias Klose wrote:

> --- src/gcc/cp/parser.c~	2009-03-11 13:02:15.000000000 +0100
> +++ src/gcc/cp/parser.c	2009-03-15 16:59:59.000000000 +0100
> @@ -2204,7 +2204,7 @@
>      {
>        /* Don't use `%s' to print the string, because quotations (`%<', `%>')
>  	 in the message need to be interpreted.  */
> -      error (parser->type_definition_forbidden_message);
> +      error ("%s", parser->type_definition_forbidden_message);
>        return false;
>      }
>    return true;

In this case, the comment explicitly tells you not to do what the patch is 
doing.

I think the right fix is to define an enumeration for all the different 
messages that can appear here, and then switch on the enumeration, passing 
the messages directly to error.  As a bonus you improve i18n, since the 
messages aren't presently marked for translation with N_().

> @@ -2291,7 +2291,7 @@
>  	  char *message = concat (thing,
>  				  " cannot appear in a constant-expression",
>  				  NULL);
> -	  error (message);
> +	  error ("%s", message);
>  	  free (message);
>  	  return true;

Again there's a comment (just above the context) telling you not to do 
this.  Again, an enumeration might be used; then you could pass full 
sentences, not concatenations, to error, and again you improve i18n.  
There are various places the C front end uses enums like this, in the 
interests of better i18n.  I reiterate my willingness to review and 
approve (as i18n maintainer) C++ front-end patches that improve i18n by 
causing full sentences to be passed for translation unless those patches 
have been specifically rejected by C++ front-end maintainers.

For the target hooks, it might be possible to change their interfaces to 
call the error functions themselves rather than passing strings about.

I hope this illustrates that if you wish to avoid these warnings for 
diagnostic functions, you need to consider carefully how to fix each case; 
you can't just apply one mechanical fix everywhere.

-- 
Joseph S. Myers
joseph@codesourcery.com