[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this! I don't think that the check should live in `misc`. It should have an alias in the `CERT` module, but maybe we want to have this live in a `posix` module? If not, this should probably live in `bugprone`.



================
Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:31
+void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto IsSigterm = [](const auto &KeyValue) -> bool {
+    return KeyValue.first->getName() == "SIGTERM";
----------------
You can drop the trailing return type on the lambda.


================
Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:35
+  const auto TryExpandAsInteger =
+      [PP = PP](Preprocessor::macro_iterator It) -> Optional<unsigned> {
+    if (It == PP->macro_end())
----------------
This lambda capture looks suspicious -- why do you need to initialize the capture?


================
Comment at: clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp:59
+    diag(MatchedExpr->getBeginLoc(),
+         "Thread should not be terminated by SIGTERM signal.");
+  }
----------------
Clang-tidy diagnostics are not supposed to be grammatically correct. I think a better way to phrase it might be something like: `thread should not be terminated by raising the 'SIGTERM' signal`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst:6
+
+Finds ``pthread_kill`` function calls when thread is terminated by 
+``SIGTERM`` signal and the signal kills the entire process, not just the
----------------
when thread is terminated by -> when a thread is terminated by raising the


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst:8
+``SIGTERM`` signal and the signal kills the entire process, not just the
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+
----------------
Why does the check not look for `SIGKILL` as well as `SIGTERM`?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69181/new/

https://reviews.llvm.org/D69181