balazske added inline comments.
Herald added a project: All.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:159-160
Itr != ItrE; ++Itr) {
const auto *CallF = dyn_cast<FunctionDecl>((*Itr)->getDecl());
- if (CallF && !isFunctionAsyncSafe(CallF)) {
- assert(Itr.getPathLength() >= 2);
- reportBug(CallF, findCallExpr(Itr.getPath(Itr.getPathLength() - 2),
*Itr),
- /*DirectHandler=*/false);
- reportHandlerCommon(Itr, SignalCall, HandlerDecl, HandlerExpr);
+ if (CallF) {
+ unsigned int PathL = Itr.getPathLength();
----------------
whisperity wrote:
>
Is this code not too long to put in the condition section?
================
Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:165-171
+ if (checkFunction(CallF, CallOrRef))
+ reportHandlerChain(Itr, HandlerExpr);
}
}
}
+bool SignalHandlerCheck::checkFunction(const FunctionDecl *FD,
----------------
whisperity wrote:
> What does the return value of `checkFunction` signal to the user, and the
> `if`?
This has a description in the header file.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler-posix.c:14
printf("1234");
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be
asynchronous-safe; calling it from a signal handler may be dangerous
[bugprone-signal-handler]
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: system call 'printf' may not be
asynchronous-safe; calling it from a signal handler may be dangerous
[bugprone-signal-handler]
}
----------------
whisperity wrote:
> I'm not exactly sure we should call `printf` (and `memcpy`) a "system call".
> As far as I can see, in this patch, `isSystemCall()` boils down to
> //"declaration in file included as system header"//
Call it "standard function"?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118370/new/
https://reviews.llvm.org/D118370
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
