[PATCH] D22742: Fix `-jobs=<N>` where <N> > 1 and the number of workers is > 1 on macOS.

[Kostya Serebryany via llvm-commits]

kcc added a reviewer: kubabrecka.
kcc added a comment.

I like the change but I am not qualified to review the Mac-specific code. 
So, please find one more reviewer for FuzzerUtilDarwin.cpp (Kuba?)


================
Comment at: lib/Fuzzer/FuzzerUtilDarwin.cpp:32
@@ +31,3 @@
+
+/* This is a reimplementation of Libc's `system()`. On Darwin the Libc
+ * implementation contains a mutex which prevents it from being used
----------------
use // comments 

================
Comment at: lib/Fuzzer/FuzzerUtilLinux.cpp:16
@@ +15,3 @@
+int ExecuteCommand(const std::string &Command) {
+  // We are assuming that Glibc was built with a re-entrant `system()` here.
+  // FIXME: How can we check for this at compile time?
----------------
please remove the comment. 
There is no problem currently and we should not be commenting on hypothetical problems that much

================
Comment at: lib/Fuzzer/test/NoAbort.cpp:10
@@ +9,3 @@
+// This is a simple function that will never abort.
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
+  if (Size > 0 && Data[0] == 'H') {
----------------
why not reuse EmptyTest.cpp? 

================
Comment at: lib/Fuzzer/test/fuzzer-jobs.test:1
@@ +1,2 @@
+# FIXME: Do this in a new directory to avoid the fuzz-N.log
+# files from clobbering other tests. We should allow the names
----------------
fix the FIXME

================
Comment at: lib/Fuzzer/test/fuzzer-jobs.test:13
@@ +12,3 @@
+# that should be created exist.
+RUN: LLVMFuzzer-NoAbort -max_total_time=4 -jobs=4 -workers=4 FuzzerJobsTestCORPUS > %t-fuzzer-jobs-test.log 2>&1 & export FUZZER_PID=$!
+# Wait a short while to give time for the child processes
----------------
will two jobs be enough?


https://reviews.llvm.org/D22742