[PATCH] D46858: Signal handling should be signal-safe

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

I found a couple more nits to pick while reading the new version of the lock-free code.  LGTM once you fix those.



================
Comment at: lib/Support/Signals.cpp:64
+    auto Desired = CallbackAndCookie::Status::Executing;
+    if (RunMe.Flag.compare_exchange_strong(Expected, Desired)) {
+      (*RunMe.Callback)(RunMe.Cookie);
----------------
Reverse condition and early continue to reduce nesting?


================
Comment at: lib/Support/Signals.cpp:80
+    auto Desired = CallbackAndCookie::Status::Initializing;
+    if (SetMe.Flag.compare_exchange_strong(Expected, Desired)) {
+      SetMe.Callback = FnPtr;
----------------
Reverse condition and early continue to reduce nesting?


================
Comment at: lib/Support/Signals.cpp:87
+  }
+  llvm_unreachable("too many signal callbacks already registered");
 }
----------------
The optimizer is free to remove this loop exit.  I think `report_fatal_error` would be more appropriate in this case.


================
Comment at: lib/Support/Unix/Signals.inc:133-134
+         Current = Current->Next.load()) {
+      if (char *OldFilename = Current->Filename.load()) {
+        if (OldFilename == Filename) {
+          // Leave an empty filename.
----------------
Early `continue`s to reduce nesting?


Repository:
  rL LLVM

https://reviews.llvm.org/D46858