[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

[Jian Cai via Phabricator via cfe-commits]

jcai19 marked 4 inline comments as done.
jcai19 added inline comments.


================
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();
----------------
george.burgess.iv wrote:
> simplicity nit: can this be a `std::string`?
replaceFunc takes a llvm::StringRef as the third parameter. StringRef is "expected to be used in situations where the character data resides in some other buffer, whose lifetime extends past that of the StringRef", which is true in this case, so I think it should be fine.


================
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.h:18
+
+/// accept() is better to be replaced by accept4().
+///
----------------
george.burgess.iv wrote:
> nit: should probably update this comment
Good catch!


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst:4
+android-cloexec-pipe
+======================
+
----------------
Eugene.Zelenko wrote:
> Please make same length as name above.
Thanks for the comments. I have fixed them accordingly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61967/new/

https://reviews.llvm.org/D61967