[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69813#1736667 <https://reviews.llvm.org/D69813#1736667>, @Charusso wrote:

> In D69813#1736611 <https://reviews.llvm.org/D69813#1736611>, @aaron.ballman wrote:
>
> > Would it make sense to use `cert.str.31.c` to remove the random dash? Would this also help the user to do something like `cert.str.*.cpp`? if they want just the CERT C++ STR rules checked? Or can they do that already even with the `-`?
>
>
> Well, we could introduce package `cert.str.c` and `cert.str.cpp` and then the rule-number follows: `cert.str.c.31` where the `31` is the name of the checker in this case, which sounds very strange. @Szelethus is the code owner of our frontend so I would wait how he imagine the layout.


I wouldn't want to go with that approach because it confuses the names from the coding standard it's meant to check. I think a good policy is to try to keep the check names similar to the coding standard names whenever possible (regardless of the coding standard).

> As I know to enable every C checker of the package `cert.str` we need to create a `c` package because we do not have such a logic to put `*` in the package name before the checker's name and the package `c` clarify what the user wants to do. Now I have checked your `cert.str.cpp` page [1] and I think the `cert.str.cpp` invocation could invoke the `cert.str.c` as a dependency, because the `c` rules apply to `cpp` as you have written.

Yes, the C++ rules incorporate some of the C rules, but not all of them, which kind of complicates things. The STR section happens to take everything from the C STR section.

> On the other hand we are trying to avoid larger scope changes than the necessary because we do not know when `cert.str.c` would contain at least two checkers. That is why I was so minimal and only introduced the package `cert` because we already have two `FLP` checkers inside the `insecureAPI` base-checker.

Understandable.

> [1] https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046330




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

https://reviews.llvm.org/D69813