[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp:24
+    : ClangTidyCheck(Name, Context),
+      RawDisallowedSeedTypes(Options.get("DisallowedSeedTypes", "")) {
+  StringRef(RawDisallowedSeedTypes).split(DisallowedSeedTypes, ',');
----------------
In order to conform to CERT's rules, I think this needs a default string of at least `time_t` and `std::time_t`.


================
Comment at: docs/ReleaseNotes.rst:60
 
+- New `cert-msc51-cpp
+  <http://clang.llvm.org/extra/clang-tidy/checks/cert-properly-seeded-random-generator.html>`_ check
----------------
You should also add a note for cert-msc32-c.


================
Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:7
+This check flags all pseudo-random number engines, engine adaptor
+instantiations and `srand()` when initialized or seeded with default argument,
+constant expression or any user-configurable type. Pseudo-random number
----------------
Please add double backticks around `srand()` instead of single backticks.


================
Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:28
+    std::time_t t;
+    engine1.seed(std::time(&t)); // Bad, system time might be controlled by user
+
----------------
Instead of using "Bad" in the comments, it would be good to use "diagnose" or "error" instead.

Note that for this one, in particular, the default behavior is to *not* diagnose, which means this check doesn't really conform to CERT's rules.


https://reviews.llvm.org/D44143