alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:25
+ callExpr(
+ callee(functionDecl(allOf(
+ isExpansionInFileMatching(HEADER_FILE), returns(asString("int")),
----------------
`functionDecl()` is implicitly `allOf`.
================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:26
+ callee(functionDecl(allOf(
+ isExpansionInFileMatching(HEADER_FILE), returns(asString("int")),
+ hasParameter(1, hasType(isInteger())),
----------------
`isExpansionInFileMatching(...)` is a dependency on the implementation of the
library. The fact that `#include <fcntl.h>` provides this macro doesn't mean
the macro is defined in this header (it could be in another transitively
included header). Same below.
================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:26
+ callee(functionDecl(allOf(
+ isExpansionInFileMatching(HEADER_FILE), returns(asString("int")),
+ hasParameter(1, hasType(isInteger())),
----------------
alexfh wrote:
> `isExpansionInFileMatching(...)` is a dependency on the implementation of the
> library. The fact that `#include <fcntl.h>` provides this macro doesn't mean
> the macro is defined in this header (it could be in another transitively
> included header). Same below.
`asString("int")` is wasteful, since it needs to allocate the string and print
the return type name there. `hasType(isInteger())` is a much better alternative.
================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:28
+ hasParameter(1, hasType(isInteger())),
+ anyOf(matchesName("open"),
matchesName("open64")))).bind("funcDecl")))
+ .bind("openFn"),
----------------
`matchesName` is wasteful, if you need just to compare the name. You probably
want to use `hasAnyName("open", "open64")`.
================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:90
+ return checkFlag(cast<IntegerLiteral>(Flags));
+ } else if (isa<BinaryOperator>(Flags)) {
+ if (cast<BinaryOperator>(Flags)->getOpcode() ==
----------------
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
================
Comment at: test/clang-tidy/android-file-descriptor.cpp:3
+
+#include <fcntl.h>
+
----------------
We don't use actual library headers in the tests. Instead, please add mock
declarations of the API you need (open, open64, O_ flags, etc.).
Repository:
rL LLVM
https://reviews.llvm.org/D33304
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
