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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
