balazske marked 3 inline comments as done.
balazske added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118
+ FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) {
+ if (isa<FunctionDecl>(CE->getCalleeDecl()))
+ CalledFunctions.push_back(CE);
+ }};
----------------
aaron.ballman wrote:
> For correctness, I think you need to handle more than just calls to function
> declarations -- for instance, this should be just as problematic:
> ```
> void some_signal_handler(int sig) {
> []{ puts("this should not be an escape hatch for the check); }();
> }
> ```
> even though the call expression in the signal handler doesn't resolve back to
> a function declaration. (Similar for blocks instead of lambdas.) WDYT?
I do not know how many other cases could be there. Probably this can be left
for future improvement, the checker is mainly usable for C code then. There is
a `clang::CallGraph` functionality that could be used instead of
`FunctionCallCollector` but the `CallExpr` for the calls is not provided by it
so it does not work for this case. Maybe there is other similar functionality
that is usable?
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.cpp:52
+void test() {
+ std::signal(SIGINT, handler_abort);
+ std::signal(SIGINT, handler__Exit);
----------------
aaron.ballman wrote:
> I'd also like to see a test case where the handler to `signal` call is itself
> not a function call:
> ```
> std::signal(SIGINT, [](int sig) {
> puts("I did the bad thing this way"); // should be diagnosed, yes?
> });
> ```
This is again a new case to handle. A new matcher must be added to detect this.
But I am not sure how many other cases are there, or is it worth to handle all
of them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87449/new/
https://reviews.llvm.org/D87449
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
