[PATCH] D33304: [clang-tidy] Add a new module Android and a new check for file descriptors.

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.


================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:65
+
+    diag(FlagArg->getLocStart(), "open(), openat(), and open64() "
+          "must include O_CLOEXEC in their flags argument.")
----------------
Use "%0 " instead of the 3 function names, so that you can use << to print the name as part of the diagnostic. You can get the name by binding the FunctionDecl in the matcher you create (and then that lets you do "FD->getName()"). You can look at something like FasterStringFindCheck.cpp for a good example of this (search for "%0").


================
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:76
+    int64_t val = aPInt.getSExtValue();
+    if((val & O_CLOEXEC) == 0)
+      return false;
----------------
Using O_CLOEXEC here is potentially a problem, since most of our compiles are cross-compiles. If O_CLOEXEC is different on the target platform than the host platform (where this code is being compiled), this check would fail. Perhaps we can get the value of O_CLOEXEC as defined for the translation unit (and if it isn't defined, that's already a pretty big indicator that any use of these functions will be wrong). Alternately, maybe just re-parsing for "O_CLOEXEC" is better.


Repository:
  rL LLVM

https://reviews.llvm.org/D33304