[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/bugprone/BugproneTidyModule.cpp:84
+    CheckFactories.registerCheck<IoFunctionsMisusedCheck>(
+        "bugprone-io-functions-misused");
     CheckFactories.registerCheck<LambdaFunctionNameCheck>(
----------------
This name reads a bit awkwardly because usually "misused" would come before "io functions". However, given that we've already said it's prone to bugs, I think "misuse" is somewhat implied. Perhaps `bugprone-io-functions` or `bugprone-misused-io-functions'?

However, I think this is actually checking an existing CERT rule and perhaps should be added to the CERT module as `cert-fio34-c` and perhaps that's the only module it should live in?


================
Comment at: clang-tidy/bugprone/IoFunctionsMisusedCheck.cpp:25-31
+              has(callExpr(callee(
+                  functionDecl(hasAnyName("::getwchar", "::getwc", "::fgetwc"))
+                      .bind("FuncDecl")))),
+              has(callExpr(callee(
+                  functionDecl(hasAnyName("::std::getwchar", "::std::getwc",
+                                          "::std::fgetwc"))
+                      .bind("FuncDecl")))),
----------------
You can combine these `hasAnyName()` calls into one given that they are otherwise identical matchers.


================
Comment at: clang-tidy/bugprone/IoFunctionsMisusedCheck.cpp:26
+              has(callExpr(callee(
+                  functionDecl(hasAnyName("::getwchar", "::getwc", "::fgetwc"))
+                      .bind("FuncDecl")))),
----------------
Why only the wide char versions and not the narrow char versions as well? For instance, those are problematic on systems where `sizeof(int) == sizeof(char)`. See https://wiki.sei.cmu.edu/confluence/display/c/FIO34-C.+Distinguish+between+characters+read+from+a+file+and+EOF+or+WEOF for more details.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42682