[PATCH] D34913: [clang-tidy] Add a new Android check "android-cloexec-socket"

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang-tidy/utils/ASTUtils.h:24
+
+/// Check whether a macro flag is present in the given argument. Only consider
+/// cases of single match or match in a binary OR expression. For example,
----------------
nit: Third-person singular form is better for interface comments ("<the function> Checks whether a macro flag .... <the function> Only considers cases of single match ....")


================
Comment at: clang-tidy/utils/ASTUtils.h:28
+bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM,
+             const LangOptions &LangOpts, const char *Flag);
 } // namespace utils
----------------
StringRef is a better type for read-only string arguments.


================
Comment at: clang-tidy/utils/ASTUtils.h:28
+bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM,
+             const LangOptions &LangOpts, const char *Flag);
 } // namespace utils
----------------
alexfh wrote:
> StringRef is a better type for read-only string arguments.
`FlagSpelling` or `FlagName` could be better than just `Flag`.


================
Comment at: docs/clang-tidy/checks/android-cloexec-socket.rst:6
+
+``socket()`` should include ``SOCK_CLOEXEC`` in its type argument to avoid the
+file descriptor leakage.
----------------
A bit more expanded explanation on where the descriptor can leak would make the documentation much better. If there's a good external information source describing this issue, you can also link there.


https://reviews.llvm.org/D34913