[PATCH] D15291: [libFuzzer] Port executing system commands to OS X

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 7 21:12:49 PST 2015


kcc added a comment.

Yay, thanks for doing this!


================
Comment at: lib/Fuzzer/FuzzerIO.cpp:97
@@ +96,3 @@
+  std::string Cmd = "base64";
+#else
+  std::string Cmd = "base64 -w 0";
----------------
This code is gone, you should not need this chunk any more, 

================
Comment at: lib/Fuzzer/FuzzerUtil.cpp:65
@@ -64,2 +64,3 @@
 int NumberOfCpuCores() {
-  FILE *F = popen("nproc", "r");
+#ifdef __APPLE__
+  const char *command = "sysctl -n hw.ncpu";
----------------
I would like to minimize the number of ifdefs in the code. 
Can we use sysctl on Linux? (If yes, just do it)

Also, if we need a simple if, something like this is better: 
// in FuzzerInternal.h:
#ifdef __APPLE__
#define LIBFUZZER_APPLE 1
#else
#define LIBFUZZER_APPLE 0
#endif

// in the code: 
command = LIBFUZZER_APPLE ? "foo_mac" : "foo_linux".


If we absolutely have to use ifdefs, make them lool like

#ifdef __APPLE__
int Foo() {
...
}
#else
int Foo() {
...
}

#endif


================
Comment at: lib/Fuzzer/FuzzerUtil.cpp:78
@@ -72,1 +77,3 @@
 int ExecuteCommand(const std::string &Command) {
+#ifdef __APPLE__
+  pid_t pid = fork();
----------------
Assuming this code works on any Posix OS, let's just use your code and not have extra ifdefs


http://reviews.llvm.org/D15291





More information about the llvm-commits mailing list