[cfe-commits] [patch] Added a suggestion when a std::string is passed to printf()

[Richard Smith]

Thanks, committed as r158887.

On Wed, Jun 20, 2012 at 5:34 PM, Sam Panzer <panzer at google.com> wrote:
> Hello again,
>
> Attached is the newest-and-improved patch.
>
> On Wed, Jun 20, 2012 at 4:46 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>>
>> Hi Sam,
>>
>> On Wed, Jun 20, 2012 at 1:31 PM, Sam Panzer <panzer at google.com> wrote:
>> > Here is the next version of this patch. Changes over the last one
>> > include
>> > Richard's suggestions for variadicArgumentPODCheck, some cleanups that
>> > were
>> > available after this adjustment, and more unification of
>> > Check{Constructor,Function,Block,ObjCMethod}Call(), to make sure that
>> > the
>> > warning is issued in a uniform way for various variadic function-like
>> > things.
>>
>> I really like this patch. A handful of quick things, then I think this
>> is ready to be checked in:
>>
>> > +  void checkSecurityProperties(NamedDecl *FDecl, Expr **Args,
>>
>> Since this is also checking PODness of vararg types, perhaps checkCall
>> would be a better name?
>
>
> Good idea; done.
>
>>
>>
>> > +  StringLiteralCheckType isFormatStringLiteral(const Expr *E,
>>
>> Since this actually does non-trivial checking work, perhaps
>> checkFormatStringExpr?
>
>
> Done.
>
>>
>>
>> > +  VariadicCallType CallType = //VariadicDoesNotApply;
>>
>> Remove this comment.
>
>
> Done.
>
>>
>>
>> > +    StringLiteralCheckType left =
>> > +    StringLiteralCheckType right =
>>
>> Left, Right.
>
>
> Fixed.
>
>>
>>
>> > +  if (isValidVarArgType(Ty) != VAK_Valid) {
>>
>> This should be == VAK_Invalid, I think.
>
>
> Yep. It was actually correct in my local source...
>
>>
>>
>> > +  const FunctionProtoType *Proto = NULL;
>> > +  if ((Proto = dyn_cast<FunctionProtoType>(FuncT))) {
>>
>> would be clearer with the dyn_cast in the initialization rather than
>> assigning in the condition.
>
>
> Done.