[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely marked 14 inline comments as done and an inline comment as not done. futogergely added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:41-42 + + // Matching the `gets` deprecated function without replacement. + auto Depr

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

whisperity added a reviewer: whisperity. whisperity added a comment. Herald added a subscriber: rnkovacs. Bump! Thanks @futogergely for picking up @ktomi996's work, and @steakhal for the help given during the implementation process! In D91000#3231417 , @m

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

martong added a comment. > I think for now it is enough to issue a warning of using these functions, and > not suggest a replacement. Should we add an option to the checker to also > check for these functions? IMHO, it is okay to start with just simply issuing the warning. Later we might add

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely added a comment. In D91000#3225369 , @balazske wrote: > The functions `asctime` and `asctime_r` are discouraged according to CERT > MSC33-C rule. These could be added to this check as well. There is a clang SA > checker `SecuritySyntaxChecker

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

steakhal added a comment. In D91000#3225369 , @balazske wrote: > The functions `asctime` and `asctime_r` are discouraged according to CERT > MSC33-C rule. These could be added to this check as well. There is a clang SA > checker `SecuritySyntaxChecker` t

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

balazske added a comment. The functions `asctime` and `asctime_r` are discouraged according to CERT MSC33-C rule. These could be added to this check as well. There is a clang SA checker `SecuritySyntaxChecker` that contains other obsolete functions (and the whole check looks like it can be done

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely added a comment. Maybe we could remove the check for setbuf() and rewind() functions, making this a pure Annex K checker. There is an overlapping with another recommendation (https://wiki.sei.cmu.edu/confluence/display/c/ERR07-C.+Prefer+functions+that+support+error+checking+over+equ

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely added a comment. "It seems like none of these projects actually use the annex K functions, which is not really a surprise. VLC and lighttpd seems to use it. @futogergely could you please run your check on those projects?" **lighttpd**: the checker issued 386 warnings. The reason is

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely added a comment. "L129 and L135 are uncovered by tests. The rest of the lines are covered by tests, according to lcov." This happens if __STDC_WANT_LIB_EXT1__ is defined empty (L129) or __STDC_WANT_LIB_EXT1__ is not literal (numeric constant, ...). CHANGES SINCE LAST ACTION https

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

aaron.ballman added a comment. Thanks for this new check! In terms of whether we should expose the check in C++: I think we shouldn't. Annex K *could* be supported by a C++ library implementation (http://eel.is/c++draft/library#headers-10), but none of the identifiers are added to namespace `s

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

steakhal added a comment. L129 and L135 are uncovered by tests. The rest of the lines are covered by tests, according to `lcov`. The checker produced in total 15 reports on these 18 projects: `memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres,tinyxml2,libwebm,xerces,bitcoin,protobuf,qt

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely marked an inline comment as done. futogergely added a comment. In D91000#3161296 , @whisperity wrote: > Should/does this work in C++ mode for `std::whatever`? Right now the checker finds the functions in the global namespace only. The recomme

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely marked an inline comment as done. futogergely added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:48 + // Matching functions with safe replacements in annex K. + auto FunctionNamesWithAnnexKReplacementMatcher = hasAnyNa

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely updated this revision to Diff 391604. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91000/new/ https://reviews.llvm.org/D91000 Files: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp clang-tools-extra/clang-tidy/cert/CMakeLists.txt clang-tools-extra/clang-tidy/cert/

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

whisperity added a comment. Should/does this work in C++ mode for `std::whatever`? Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:48 + // Matching functions with safe replacements in annex K. + auto FunctionNamesWithAnnexKReplacementMatcher = hasA

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely updated this revision to Diff 390309. futogergely added a comment. x64 debian failed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91000/new/ https://reviews.llvm.org/D91000 Files: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp clang-tools-extra/clang-tidy/cert/CM

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

steakhal resigned from this revision. steakhal added a comment. I've already reviewed this change offline, thus I resign. For me this looks great, I'm eager to hear your opinions! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91000/new/ https://reviews.llvm.org/D91000

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

futogergely updated this revision to Diff 390270. futogergely retitled this revision from "[clang-tidy] CERT MSC24-C Obsolescent Functions check" to "[clang-tidy] Add cert-msc24-c checker.". futogergely edited the summary of this revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D910