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

[Richard Smith]

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?

> +  StringLiteralCheckType isFormatStringLiteral(const Expr *E,

Since this actually does non-trivial checking work, perhaps
checkFormatStringExpr?

> +  VariadicCallType CallType = //VariadicDoesNotApply;

Remove this comment.

> +    StringLiteralCheckType left =
> +    StringLiteralCheckType right =

Left, Right.

> +  if (isValidVarArgType(Ty) != VAK_Valid) {

This should be == VAK_Invalid, I think.

> +  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.