The categories are supposed to be very broad groupings: "Logic error", "Memory leak", "Convention X". "Logic error" is probably fine here, even though it's generic, since there's not really a group that fits this general kind of mistake. I just want to avoid magic strings—we shouldn't have variants of the same thing because they'll show up as different categories.
Comments on the patch: + // Only check add operators. + if (B->getOpcode() != BO_Add) How about +=? + const CharacterLiteral *CharExpr = + dyn_cast<CharacterLiteral>(B->getRHS()->IgnoreImpCasts()); Should this be a character literal, or just any expression with "char" type? I realize that someone could be using a char-sized variable as an index, but that seems fairly rare, and harder to catch by hand. + if (!(StringRefExpr->getType() == + Context.getPointerType(Context.getConstType(Context.CharTy)) || + StringRefExpr->getType() == Context.getPointerType(Context.CharTy))) Things to consider: 1. Do explicitly signed or unsigned chars count? In that case, you should use Type::isCharType(). 2. Do wide chars count? Then you should use Type::isAnyCharacterType(). 3. Even if none of the above, you should consider typedefs, so it's still worth going through getCanonicalType() and getPointeeType() rather than building up a new pointer type. One possibility: StringRefExpr->getType()->getPointeeType()->isAnyCharacterType(). At this point, though, StringRefExpr->getType() probably deserves a helper variable. +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.StringPlusChar -verify %s Please include the "core" package in these checks; running without core enabled isn't a use case we're interested in. Jordan On Sep 30, 2013, at 3:25 , Anders Rönnholm <[email protected]> wrote: > Hi Jordan, > > Thank you for your comments. I have changed the check to use > RecursiveASTVisitor. However i'm not sure what i should write as category in > bugreport. You mentioned adding a new category in CommonBugCategories for > SIzeofOnExpression, should i do that or is what i have enough for now? > > I have provided a new diff. > > //Anders
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
