[PATCH] D40103: [libFuzzer] Encapsulate commands in a class

Kostya Serebryany via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 16 10:37:57 PST 2017


kcc added a reviewer: morehouse.
kcc added a comment.

Nice, thank you! 
Do I understand correctly that this is a no-functional-change-intended change?

Some comments below. 
Matt, please make the next round of reviews (once my comments are addressed) and land it when done. (Assuming Aaron doesn't have commit access).



================
Comment at: lib/fuzzer/FuzzerCommand.cpp:53
+  std::stringstream SS;
+  for (auto arg : Args) {
+    SS << arg << " ";
----------------
here and below:
LLVM coding style prefers to not use {} for single-statement if-body


================
Comment at: lib/fuzzer/FuzzerCommand.h:21
+namespace fuzzer {
+class Command final {
+public:
----------------
consider haing all of this class in the header. There is not too much code in it and splitting it into t .h and .cpp bloats the code. 


================
Comment at: lib/fuzzer/FuzzerCommand.h:22
+class Command final {
+public:
+  Command();
----------------
Does this class deserve a gunit-style unit test in lib/fuzzer/tests/FuzzerUnittest.cpp ?


https://reviews.llvm.org/D40103





More information about the llvm-commits mailing list