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

[Kuba Brecka via llvm-commits]

kubabrecka added a comment.

A bunch of nits.  There really isn’t anything Darwin-specific in the FuzzerUtilDarwin.cpp file, it’s just POSIX code.


================
Comment at: lib/Fuzzer/FuzzerUtilDarwin.cpp:24-27
@@ +23,6 @@
+static std::mutex SignalMutex;
+// Global varaibles used to keep track of how
+// signal handling should be restored. They
+// should **not** be accessed without holding
+// `SignalMutex`.
+static int UseCount = 0;
----------------
This can fit just two lines. Typo “varaibles”.

================
Comment at: lib/Fuzzer/FuzzerUtilDarwin.cpp:28
@@ +27,3 @@
+// `SignalMutex`.
+static int UseCount = 0;
+static struct sigaction OldSigIntAction;
----------------
Can you please come up with a better name than “UseCount"?

================
Comment at: lib/Fuzzer/FuzzerUtilDarwin.cpp:40-47
@@ +39,10 @@
+int ExecuteCommand(const std::string &Command) {
+  const char *CommandCStr = Command.c_str();
+  pid_t Pid;
+  int ErrorCode = 0, ProcessStatus = 0;
+  char **Environ = environ; // Read from global
+  const char *Argv[] = {"sh", "-c", CommandCStr, NULL};
+  sigset_t DefaultSigSet;
+  posix_spawnattr_t SpawnAttributes;
+  short SpawnFlags = POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK;
+  (void)sigemptyset(&DefaultSigSet);
----------------
Can we move these variables to the places where they’re first used?

================
Comment at: lib/Fuzzer/FuzzerUtilDarwin.cpp:77
@@ +76,3 @@
+          -1) {
+        Printf("Failed to blcok SIGCHLD\n");
+        // Try our best to restore the signal handlers.
----------------
typo

================
Comment at: lib/Fuzzer/FuzzerUtilDarwin.cpp:89-94
@@ +88,8 @@
+  // following signals rather than inheriting what the parent has.
+  (void)sigaddset(&DefaultSigSet, SIGQUIT);
+  (void)sigaddset(&DefaultSigSet, SIGINT);
+  (void)posix_spawnattr_setsigdefault(&SpawnAttributes, &DefaultSigSet);
+  // Make sure the child process doesn't block SIGCHLD
+  (void)posix_spawnattr_setsigmask(&SpawnAttributes, &OldBlockedSignalsSet);
+  (void)posix_spawnattr_setflags(&SpawnAttributes, SpawnFlags);
+
----------------
Not a huge fan of the "(void)" prefixes.  I understand why you put them there, but no other code in libFuzzer seems to be using them.  Does this really produce any warnings?


https://reviews.llvm.org/D22742