steakhal added a comment.

In D91000#2469884 <https://reviews.llvm.org/D91000#2469884>, @lebedev.ri wrote:
> In D91000#2469880 <https://reviews.llvm.org/D91000#2469880>, @steakhal wrote:
>
>> 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.
>
> I suspect that should be a flag in the check's options.

I don't see any value in disabling the check for eg. `setbuf`. Even the man 
page says that you should use the `setvbuf` instead.
Why would anyone disable this if one already wants diagnostics for the CERT 
rule it implements?

>> 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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to