njames93 added a comment.
Should this check be renamed to bugprone-sprintf-with-fixed-size-buffer?
Have you thought about an option to flag all calls to sprintf as they are
typically not recommended by a lot of coding practice guides. Obviously we
couldn't generate a fix-it as the buffer size may be unknown, but it could help
fix someone's buffer overflow exploit.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp:23
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store<bool>(Opts, "PreferSafe", PreferSafe);
+}
----------------
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp:29
+ auto HasFixedSizeBufferMatcher =
+ hasArgument(0, anyOf(memberExpr(HasConstantArrayType).bind("MemberExpr"),
+ declRefExpr(HasConstantArrayType).bind("DeclRef")));
----------------
Check out the `mapAnyWith` matcher, would simply a lot of this code.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp:33
+ Finder->addMatcher(callExpr(allOf(HasFixedSizeBufferMatcher,
+
callee(functionDecl(hasName("::sprintf")))))
+ .bind("sprintf"),
----------------
Just use `hasAnyName("::sprintf", "::vsprintf")`, Then in the check you can
call `getName` to find out which one.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp:58-70
+ std::string Recommendation;
+ if (Call = Result.Nodes.getNodeAs<CallExpr>("sprintf"))
+ Recommendation = "snprintf";
+ else if (Call = Result.Nodes.getNodeAs<CallExpr>("vsprintf"))
+ Recommendation = "vsnprintf";
+
+ assert(Call && "Unhandled binding in the Matcher");
----------------
Avoid string manipulation where possible.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h:27
+ : ClangTidyCheck(Name, Context),
+ PreferSafe(Options.get<bool>("PreferSafe", false)) {}
+
----------------
Let template deduction handle this.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h:30
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
----------------
This check seems like it would have value in C code as well as C++.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132294/new/
https://reviews.llvm.org/D132294
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
