lebedev.ri added a comment. In D91000#2469894 <https://reviews.llvm.org/D91000#2469894>, @steakhal wrote:
> 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. I think the question is, *why* are these checks being implemented? Just to claim that for some particular rule there is a check, and cross it off a list? Or for them to be actually used? 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
