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

[Jian Cai via Phabricator via cfe-commits]

jcai19 marked 2 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:
> jcai19 wrote:
> > 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.
> Both should be functionally equivalent in this code. My point was that it's not common to rely on temporary lifetime extension when writing the non-`const&` type would be equivalent (barring maybe cases with `auto`, but that's not applicable), and I don't see why we should break with that here.
Make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61967