[cfe-commits] [Patch] Warn about non-standard format strings (PR12017)

[Ted Kremenek]

On Feb 20, 2012, at 11:35 AM, Joerg Sonnenberger <joerg at britannica.bec.de> wrote:

> On Mon, Feb 20, 2012 at 11:02:25AM -0800, Ted Kremenek wrote:
>> (1) Warn about non-standard format strings only when it is an issue
>> for the target platform.  This is hard to do, but this is basically
>> what users will expect.
> 
> IMO most of the format extensions are of questionable value (%m),
> outdated historic extensions (q modifier) or just plainly unportable (S
> modifier).

I'm not questioning the motivation for these changes; I just don't think your argument is taking into account the implications of these warning changes on existing code.  It's worth fighting the fight to change code when it has real value, but in my users get really testy about warnings that tells them something is wrong with their code when that isn't really the case.

> Allowing them by default makes it a lot easier to slip them
> unintentionally in for no real value in case of the first two
> categories.

The value is not pissing off users with useless warnings if they don't really apply to them.  I'm not drawing a line in the sand here, but it's is important to keep in mind that there is a lot of code out there that has treated existing format string checking as "API" and will be dubious about us being more pedantic just for the sake of being pedantic.

> The third category is even more painful since it can require
> extensive code changes. Worse, even a single target playform is not
> necessarily consistent.

Yes, this is all a mess.

> Consider 'a' handling for scanf. Many embeddeed
> systems use Linux with uClibc -- which doesn't implement it. How do you
> detect if the libc support it or not?


I wish I had good answers to these questions.  I don't, and that's my concern.  What I do know is that I have reviewed bug reports from users who complained about Clang warning about format specifiers that were okay in their case (and their case wasn't that special), which is why some of these extensions got added to Clang in the first place.

I'm all for catching bugs, but my concern is issuing too many warnings that people will get really pissed off, or worse, turn the warning off.  If we are going to tighten the bolts on format string checking, I think we need a solution that either allows Clang to make accurate decisions, or provide a way for users to opt into stricter checking.  Stricter checking that doesn't apply to most users that provides marginal value is not going to be popular.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120220/9ed98c4b/attachment.html>