george.burgess.iv added a comment.
Thanks for this!
I don't have great context on tidy, so I can't stamp this, but I do have a few
drive-by nits for you.
================
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:22
+ functionDecl(returns(isInteger()), hasName("pipe"),
+ //hasParameter(0,
hasType(constantArrayType(hasElementType(isInteger()), hasSize(2)))))
+ hasParameter(0,
hasType(pointsTo(isInteger())))));
----------------
We probably don't want to commit commented out code
================
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:27
+void CloexecPipeCheck::check(const MatchFinder::MatchResult &Result) {
+ const std::string &ReplacementText =
+ (Twine("pipe2(") + getSpellingArg(Result, 0) + ", O_CLOEXEC)").str();
----------------
simplicity nit: can this be a `std::string`?
================
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h:18
+
+/// accept() is better to be replaced by accept4().
+///
----------------
nit: should probably update this comment
================
Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:9
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer pipe2() to pipe() because
pipe2() allows O_CLOEXEC [android-cloexec-pipe]
+ // CHECK-FIXES: pipe2(pipefd, O_CLOEXEC);
+}
----------------
(Do we have a CHECK-FIXES-NOT or CHECK-MESSAGES-NOT to apply to the things
below? Or are the CHECKs here meant to be complete like clang's `-verify`?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61967/new/
https://reviews.llvm.org/D61967
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
