[cfe-commits] [patch] -Wformat fixup for vprintf (and friends)

[Jean-Daniel Dupas]

Thanks,

What do you mean by "the right thing".

In the C test, the function check_string_literal2() test that the warning does not trigger when the user forward a format string argument.

In the C++ test, the "int Foo::printf(const char *fmt, …)" contains a case with vprintf() and no warning, and finally, the objc test case contains a similar test (+ [Bar log2:(NSString *)fmt] ) with NSLogv().


Le 21 févr. 2012 à 18:49, Ted Kremenek a écrit :

> Hi Jean-Daniel,
> 
> Overall this looks good.  Do we have test cases for this not firing on vprintf, etc., when the user does the right thing?  I didn't notice anything in the patch.  There may be some existing test cases, but it's probably worth showing that purely correct behavior doesn't trigger a warning.
> 
> Ted
> 
> On Feb 20, 2012, at 10:23 AM, Jean-Daniel Dupas <devlists at shadowlab.org> wrote:
> 
>> ping.
>> 
>> Le 16 févr. 2012 à 23:39, Jean-Daniel Dupas a écrit :
>> 
>>> 
>>> Le 16 févr. 2012 à 19:17, Ted Kremenek a écrit :
>>> 
>>>> On Feb 16, 2012, at 1:57 AM, Jean-Daniel Dupas <devlists at shadowlab.org> wrote:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> This patch implements a long standing FIXME.
>>>>> When calling a non variadic format function(vprintf, vscanf, NSLogv, …), is the format argument references a parameter of the enclosing function, clang never warned.
>>>>> With this change, clang will inhibit the warning only if the parameter is declared as a format string with a compatible type.
>>>>> 
>>>>> 
>>>>> -- Jean-Daniel
>>>>> 
>>>> 
>>>> Hi Jean-Daniel,
>>>> 
>>>> I've noticed this patch contains seemingly unrelated changes, e.g.:
>>>> 
>>>> 
>>>>> // RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral -pedantic %s
>>>>> 
>>>>> +#include <stdarg.h>
>>>>> +
>>>>> extern "C" {
>>>>> -extern int scanf(const char *restrict, ...);
>>>>> -extern int printf(const char *restrict, ...);
>>>>> +extern int scanf(const char *, ...);
>>>>> +extern int printf(const char *, ...);
>>>>> +extern int vprintf(const char *, va_list ap);
>>>>> }
>>>>> 
>>>> 
>>>> 
>>>> What is the motivation for removing the 'restrict' in these test cases?  It seems completely unrelated, unnecessary, and actually incorrect since clang defaults to c99.
>>> 
>>> My bad.
>>> I noticed that the restrict keyword was not supported in C++ member declaration and was interpreted as the parameter name instead, so I removed it to avoid confusion, and I erroneously removed it from the C functions too.
>>> 
>>> Is this new patch OK ?
>>> 
>>> -- Jean-Daniel
>>> 
>>> 
>>> 
>>> <vprintf.patch>_______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> 
>> -- Jean-Daniel
>> 
>> 
>> 
>> <vprintf.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-- Jean-Daniel