[PATCH] D35372: [clang-tidy] Refactor the code and add a close-on-exec check on memfd_create() in Android module.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

Looks good to me, a few nits. Thanks for improving it continuously.

I'd hold it for a while to see whether @alexfh has further comments before submitting it.



================
Comment at: clang-tidy/android/CloexecCheck.h:28
+/// prevent the file descriptor leakage on fork+exec and this class provides
+/// utilities to identify  and fix these C functions.
+class CloexecCheck : public ClangTidyCheck {
----------------
remove an extra blank before `and`.


================
Comment at: clang-tidy/android/CloexecCheck.h:45
+  ///   open(file, O_RDONLY | O_CLOEXE);
+  /// \endcode
+  void insertMacroFlag(const ast_matchers::MatchFinder::MatchResult &Result,
----------------
Nit: you are missing the parameter part in the comment, the same below.

```
/// \param Result XXX
/// \param MacroFlag XXX
/// \param ArgPos
```


================
Comment at: clang-tidy/android/CloexecCheck.h:47
+  void insertMacroFlag(const ast_matchers::MatchFinder::MatchResult &Result,
+                       const StringRef MarcoFlag, const int ArgPos);
+
----------------
Is the `ArgPos` 0-based or 1-based? I know it is 0-based, but we'd better mention in the document part.


================
Comment at: clang-tidy/android/CloexecCheck.h:116
+  template <typename... Types>
+  void registerMatchersImpl(ast_matchers::MatchFinder *Finder,
+                            Types... Params) {
----------------
nit: I'd put this method as the first protected method.


https://reviews.llvm.org/D35372