[PATCH] D19590: Check for CERT ERR34-C. Detect errors when converting a string to a number
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 29 09:39:45 PDT 2016
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good with a few nits.
================
Comment at: clang-tidy/cert/StrToNumCheck.cpp:182
@@ +181,3 @@
+void StrToNumCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *CE = Result.Nodes.getNodeAs<CallExpr>("expr");
+ const FunctionDecl *FD = nullptr;
----------------
Too many abbreviations for my taste. How about CE -> Call, FD -> Function or FuncDecl, CK -> Conversion, CFD -> ConverterFunc?
================
Comment at: clang-tidy/cert/StrToNumCheck.cpp:224
@@ +223,3 @@
+
+ std::string ConversionPerformed = ClassifyConversionType(CK),
+ AlternateFunction = ClassifyReplacement(CK);
----------------
1. s/std::string/StringRef/
2. I'd make the functions return `StringRef` instead of a `const char *`
3. One variable per declaration, please.
4. I'm not sure the variables help making the code easier to read.
================
Comment at: docs/clang-tidy/checks/cert-err34-c.rst:9
@@ +8,3 @@
+does not flag calls to ``strtol()``, or other, related conversion functions that
+do perform better error checking.
+
----------------
Maybe add a couple of examples?
http://reviews.llvm.org/D19590
More information about the cfe-commits
mailing list