[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