[PATCH] D118370: [clang-tidy] bugprone-signal-handler: Message improvement and code refactoring.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:146
+  if (!HandlerDecl->hasBody()) {
+    checkFunction(HandlerDecl, HandlerExpr);
     return;
----------------
Can we put `(void) checkFunction(...)` here to make it clear
we're intentionally ignoring the return value?


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:36
 private:
-  bool isFunctionAsyncSafe(const FunctionDecl *FD) const;
+  /// Check if a function is allowed as signal handler.
+  /// Should test properties of the function, and check in the code body.
----------------
... `as a signal handler`


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:37
+  /// Check if a function is allowed as signal handler.
+  /// Should test properties of the function, and check in the code body.
+  /// Should not check function calls in the code (this part is done by the call
----------------
`Should test the properties` ...


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:42
+  /// @param CallOrRef Location of the call to this function (in another
+  /// function) or the reference to the function (if it is used as registered
+  /// signal handler). This is the location where diagnostics are to be placed.
----------------
... `as a registered`


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:44
+  /// signal handler). This is the location where diagnostics are to be placed.
+  /// @return true only if problem was found in the function.
+  bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef);
----------------
... `only if a problem` ...


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:48
   bool isSystemCallAsyncSafe(const FunctionDecl *FD) const;
-  void reportBug(const FunctionDecl *CalledFunction, const Expr *CallOrRef,
-                 bool DirectHandler);
-  void reportHandlerCommon(llvm::df_iterator<clang::CallGraphNode *> Itr,
-                           const CallExpr *SignalCall,
-                           const FunctionDecl *HandlerDecl,
-                           const Expr *HandlerRef);
+  /// Add bug report notes to show the call chain of functions from signal
+  /// handler to an actual called function (from it).
----------------
... `from a signal`


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:51
+  /// @param Itr Position during a call graph depth-first iteration. It contains
+  /// the "path" (call chain) from signal handler to the actual found function
+  /// call.
----------------
.. `from the signal handler` ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118370