whisperity added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:45-50
+// The following functions are
+// deliberately excluded because they can
+// be called with NULL argument and in
+// this case the check is not applicable:
+// mblen, mbrlen, mbrtowc, mbtowc, wctomb,
+// wctomb_s
----------------
aaron.ballman wrote:
> Pretty sure this comment can be re-flowed to 80 columns. Also needs trailing
> punctuation.
Shouldn't we reuse `utils::options::serializeStringList` here instead of
hardcoding the separator character into a giant literal? I know executing that
operation has an associated run-time cost, and as it is not `constexpr` it
needs to be done somewhere else, and `std::string` could throw so we can't do
it at "static initialisation" time... But having those extra chars there just
seems way too fragile at a later modification. We've had cases where people
missed separating `,`s even -- and those are syntactically highlighted
differently due to being outside the string literal.
Suggestion:
* An array of `StringRef`s or even `llvm::StringLiteral`s containing the
individual function names. Array separated with `,`, the separator outside the
literal. Aligned to column and probably one per line.
* When using this variable later (🌟), instead of passing the stringref, pass
the result of `serializeStringList`.
================
Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:325
Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "false";
+ Opts["cert-err33-c.CheckedFunctions"] = CertErr33CCheckedFunctions;
return Options;
----------------
(🌟)
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst:11
+
+* aligned_alloc()
+* asctime_s()
----------------
`mblen`, `mbrlen`, etc. are with backticks later. But this list isn't. Was this
intended?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112409/new/
https://reviews.llvm.org/D112409
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
