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

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tidy/utils/CloexecFlagChecker.h:21
+/// specific macro flag in a given argument.
+bool hasCloseOnExecFlag(const Expr *Flags, const SourceManager &SM,
+                        const LangOptions &LangOpts,
----------------
Maybe put this function to `utils/ASTUtils.h` file.

The name `CloseOnExecFlag` is too specific, it actually checks whether the `flags` expr has a given flag IMO, might be call it `hasFlag` (a better name are welcome), and document more details of the API in the above comment.


================
Comment at: test/clang-tidy/android-cloexec-socket.cpp:20
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'socket' should use SOCK_CLOEXEC where possible [android-cloexec-socket]
+  // CHECK-FIXES: SOCK_STREAM | SOCK_CLOEXEC
+  TEMP_FAILURE_RETRY(socket(0, SOCK_STREAM, 0));
----------------
I'd use the complete statement for checking the fixes here, the same below.


================
Comment at: test/clang-tidy/android-cloexec-socket.cpp:92
+};
+
----------------
nit: remove the empty line.


Repository:
  rL LLVM

https://reviews.llvm.org/D34913