Quuxplusone added inline comments.

================
Comment at: docs/clang-tidy/checks/cert-properly-seeded-random-generator.rst:26
+    std::random_device dev;
+    std::mt19937 engine3(dev()); // Good
+  }
----------------
Seeding MT19937 with a single 32-bit integer is //not// "Good". It makes the 
seed super easy to brute-force; and for example, `engine3` will never produce 7 
or 13 as its first output.
http://www.pcg-random.org/posts/cpp-seeding-surprises.html

This doesn't affect the implementation or usefulness of this clang-tidy check, 
which is pretty nifty. I merely object to marking this sample code with the 
comment "Good" in official documentation. It should be marked "Will not warn" 
at best. Or replace it with something slightly more realistic, e.g.

    int x = atoi(argv[1]);
    std::mt19937 engine3(x);  // Will not warn

As Aaron said above, seeding with the current time is approximately as good an 
idea, and "will not warn" with the current diagnostic either.

The correct way to seed a PRNG is to initialize the //entire state// with 
random bits, not just 32 bits of the state. This can be done, but not yet in 
standard C++: 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0205r0.html


================
Comment at: test/clang-tidy/cert-properly-seeded-random-generator.cpp:76
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator must be 
seeded with a random_device instead of a constant 
[cert-properly-seeded-random-generator]
+  engine1.seed(seed);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: random number generator must be 
seeded with a random_device instead of a constant 
[cert-properly-seeded-random-generator]
----------------
Is the diagnostic suppressed if `seed` is a template parameter? (Not that I'd 
do this. It's just a corner case I thought of.)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44143



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to