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

Jian Cai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 2 15:44:26 PDT 2019

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

Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31
+      Result,
+      "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC",
+      ReplacementText);
gribozavr wrote:
> hokein wrote:
> > the message doesn't seem to explain the reason, especially to the people who are not familiar with the `pipe` API, maybe `prefer pipe2() to pipe() to avoid file descriptor leakage` is clearer?
> > 
> > Ah, it looks like you are following the existing `CloexecCreatCheck` check, no need to do it in this patch. 
> "prefer pipe2() with O_CLOEXEC to avoid leaking file descriptors to child processes"
> No need to change in this patch if you're following an existing pattern, but please do update the message in all checks in a separate patch to explain things better.
Thanks for the comments. I have updated the text. 

Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:17
+  pipe(pipefd);
+  // CHECK-MESSAGES-NOT: warning:
srhines wrote:
> hokein wrote:
> > nit: no need to do it explicitly, if a warning is shown unexpectedly, the test will fail.
> I somehow never realized this (and there seem to be several places that also don't realize it). Thanks for letting us know.
Thanks for the clarification.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list