This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: "format not a string literal"


 > From: Eli Zaretskii <eliz@gnu.org>
 > 
 > >  >   printf (n == 1 ? "%d file" : "%d files", n);
 > > 
 > > An easy to fix bad behavior is not a reason to change the warning,
 > > even if lots of people do it.  This is especially true if the warning
 > > points exactly to the place where the problem occurs and the fix is
 > > immediately clear.
 > > 
 > > Note I submitted patches to fix the cases in gcc which did the above
 > > ?: silliness as well as several array format string usages.  The
 > > warning in those cases is entirely legitimate.
 > 
 > Could you please explain why this practice is ``silly'', and why does
 > it deserve a warning?  As someone who uses such code, perhaps out of
 > utter ignorance, I'd like to know why I will be punished by the new
 > versions of GCC.
 > 
 > In particular, it would be nice to know whether such code violates
 > some established standards, and if so, which ones.
 > Thanks.

Please do not consider this as punishment, it is a benefit.  Legal C
is not necessarily good C and the warning code is full of stuff which
attempts to encourage better practices which produce code which is
less bug prone and more legible and maintainable.  Not everyone agrees
on what is legible and maintainable, so I'll stick to where this helps
gcc find bugs.

The new code tells you where -Wformat has been and still is unable to
validate your printf formats.  The ?: case is one of them.  If you
write

 > printf (n == 1 ? "%d file" : "%d files", n);

You're just simply not getting a format check here.  (One could argue
the warning should be rewritten to handle this, but there are many
such cases and this work would never be done.)

Anyway, imagine if you had written:

 > printf (n == 1 ? "%d file %s" : "%d files", n);

This code would be accepted by gcc -Wformat and core dump when
executed with n == 1.

You should instead write it as 

 > if (n == 1)
 >   printf ("%d file", n);
 > else
 >   printf ("%d files", n);

Then gcc can check everything.

Also according to Ulrich, calling ?: in a format string is not good
from an intl perspective.  So in general its best to fix these cases.

		--Kaveh
--
Kaveh R. Ghazi			Engagement Manager / Project Services
ghazi@caip.rutgers.edu		Qwest Internet Solutions


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]