Eugene.Zelenko added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:19 +namespace { +static Preprocessor *PP; +} ---------------- Why this could not be member of check class? ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:52 + bool AnnexKIsWanted; + if (!PP->isMacroDefined("__STDC_LIB_EXT1__")) { + AnnexKIsAvailable = false; ---------------- Why not to use plain assignment? See `readability-simplify-boolean-expr`. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:59 + AnnexKIsWanted = false; + if (!Id) { + AnnexKIsWanted = false; ---------------- Early return will be better. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:69 + AreSafeFunctionsWanted = IntValue.getZExtValue(); + if (AreSafeFunctionsWanted.hasValue()) { + AnnexKIsWanted = AreSafeFunctionsWanted.getValue(); ---------------- Please elide braces. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:80 + bool AnnexKIsWanted = std::get<1>(AnnexKUsage); + if (!AnnexKIsWanted || !AnnexKIsAvailable) { + return; ---------------- Please elide braces. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:84 + if (const auto *Function = Result.Nodes.getNodeAs<CallExpr>("callexpr")) { + const FunctionDecl *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn"); + const NamedDecl *NFD = dyn_cast<NamedDecl>(FD); ---------------- Could be `const auto *`, because type is spelled in same statement. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:85 + const FunctionDecl *FD = Result.Nodes.getNodeAs<FunctionDecl>("fn"); + const NamedDecl *NFD = dyn_cast<NamedDecl>(FD); + std::string FunctionName = NFD->getNameAsString(); ---------------- Could be `const auto *`, because type is spelled in same statement. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:92 + if (const auto *FunctionPointer = Result.Nodes.getNodeAs<VarDecl>("fp")) { + if (const FunctionDecl *FD = Result.Nodes.getNodeAs<FunctionDecl>("fnp")) { + const NamedDecl *NFD = dyn_cast<NamedDecl>(FD); ---------------- Could be `const auto *`, because type is spelled in same statement. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:93 + if (const FunctionDecl *FD = Result.Nodes.getNodeAs<FunctionDecl>("fnp")) { + const NamedDecl *NFD = dyn_cast<NamedDecl>(FD); + std::string FunctionName = NFD->getNameAsString(); ---------------- Could be `const auto *`, because type is spelled in same statement. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h:12 + +#include "../ClangTidyCheck.h" +namespace clang { ---------------- Please separate include statement and namespaces with empty line. ================ Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.h:13 +#include "../ClangTidyCheck.h" +namespace clang { +namespace tidy { ---------------- Please separate with empty line. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-obsolescent-functions.rst:6 + +Guards against using some unsafe function calls and function pointers which initialized with unsafe functions if __STDC_LIB_EXT1__ macro is defined and the value of __STDC_WANT_LIB_EXT1__ is 1. The usage of following functions are checked : bsearch, + fprintf, fscanf, fwprintf, fwscanf, getenv, gmtime, localtime, mbsrtowcs, ---------------- Please make first statement same as in Release Notes. Please enclose all preprocessor variables and function names in double back-ticks. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-obsolescent-functions.rst:15 + +This is a CERT security rule: +https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions ---------------- Link to original rule should be at the end. See other documentation as example. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-obsolescent-functions.rst:17 +https://wiki.sei.cmu.edu/confluence/display/c/MSC24-C.+Do+not+use+deprecated+or+obsolescent+functions + Example : + .code-block:: ---------------- Please remove space before colon and separate code with empty line. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-obsolescent-functions.rst:18 + Example : + .code-block:: + #define __STDC_WANT_LIB_EXT1__ 1 ---------------- Should be `.. code-block:: c++`. See other documentation as formatting example. ================ Comment at: clang-tools-extra/test/clang-tidy/cert-obsolescent-functions.cpp:5 +#define __STDC_WANT_LIB_EXT1__ 1 +void * memmove(void *, void *, unsigned int); +void f1(const char *in) { ---------------- Please separate with empty line. 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