[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

Wed Aug 5 05:05:27 PDT 2020

whisperity added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64
+/// argument.
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsRegisterFunction =
----------------
aaron.ballman wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > whisperity wrote:
> > > > What happens if this test is run on C++ code calling the same functions? I see the rule is only applicable to C, for some reason... Should it be disabled from registering if by accident the check is enabled on a C++ source file?
> > > The CERT C++ rules inherit many of the C rules, including this one. It's listed towards the bottom of the set of inherited rules here: https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046336
> > Right, thanks for the heads-up. This should be somewhat more indicative on the Wiki on the page for the rule (especially because people won't immediately understand why a `-c` check reports on their cpp code, assuming they understand `-c` means C.)
> I would imagine people shouldn't be confused by a C check triggering in C++ given that C++ incorporates much of C so there's a considerable amount of overlap (for instance, this hasn't been an issue with `cert-env33-c` which applies in both C and C++).
> 
> That said, what do you think should be indicated on the wiki (I assume you mean the CERT wiki and not the clang-tidy documentation)? I'm happy to pass the suggestion along to folks I still know at CERT.
> That said, what do you think should be indicated on the wiki (I assume you mean the CERT wiki and not the clang-tidy documentation)?

On the page of a C rule "ABC-00", if it also applies for C++, it should be indicated. Just this fact: //"In addition, this rule is applicable for C++ programs [bla bla]."// where `[bla bla]` is something like "calling C standard library operations" or "dealing with C API" or "handling C data structures" or simply no extra "elaboration", depending on what the rule is targeting.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717