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

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D69813#1734193 <https://reviews.llvm.org/D69813#1734193>, @Szelethus wrote:

> Hmm, so this checker is rather a collection of CERT rule checkers, right? Shouldn't the checker name contain the actual rule name (STR31-C)? User interfacewise, I would much prefer smaller, leaner checkers than a big one with a lot of options, which are barely supported for end-users. I would expect a `cert` package to contain subpackages like `cert.str`, and checker names `cert.str.31StringSize`, or similar.


It is the STR rules of CERT, nothing else. Most of the rules are tied together, and that is why the checker needs to be designed as one checker at first. I am not sure which part of the STR I will cover, so may when the checker evolves and some functions does not need the same helper methods we need to create new checkers. STR31 and STR32 are my projects which is like one single project. Also I did not except the users to specify the rule number, but this checker could be something like `cert.str.Termination`. There is two floating-point CERT checkers inside the `insecureAPI` that is why I have introduced the `cert` package, which will have three members, one is that new checker. I think a new package is only necessary if it contains at least two checkers.

> Also, shouldn't we move related checkers from `security.insecureAPI` to `cert`? Or just mention the rule name in the description, and continue not having a  `cert` package?

We should not, they does not fit into CERT rules, but it has two CERT floating-point checkers. The `cert` package should be well described with CERT rules. I want to move the CERT checkers from it into that `cert` package, and leave the rest.


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

https://reviews.llvm.org/D69813