[PATCH] D91000: [clang-tidy] CERT MSC24-C Obsolescent Functions check

Wed Dec 23 04:38:27 PST 2020

steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

In D91000#2465514 <https://reviews.llvm.org/D91000#2465514>, @ktomi996 wrote:

> In D91000#2382562 <https://reviews.llvm.org/D91000#2382562>, @steakhal wrote:
>
>> Quoting the revision summary:
>>
>>> This checker guards against using some vulnerable C functions which are mentioned in MSC24-C in obsolescent functions table.
>>
>> Why don't we check the rest of the functions as well?
>> `asctime`, `atof`, `atoi`, `atol`, `atoll`, `ctime`, `fopen`, `freopen`, `rewind`, `setbuf`
>>
>> Hm, I get that `cert-err34-c` will already diagnose the uses of `atof`, `atoi`, `atol`, `atoll`, but then why do we check `vfscanf`, `vscanf` then?
>> We should probably omit these, while documenting this.
>> On the other hand, I would recommend checking `asctime`, `ctime`, `fopen`, `freopen`, `rewind`, `setbuf` for the sake of completeness.
>
> I only check functions which are in Unchecked Obsolescent Functions without setbuf because setbuf does not have a safer alternative in Annex K.
> https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions

>From the user's point of view, it does not matter if the //safer alternative// is inside //annex K// or not.
IMO if the //CERT// rule says that don't use any of these functions but use these //other// functions instead.
If we don't check all of them in the list, this checker is incomplete. The name of this checker might lead the user to a false sense of security.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91000