[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