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

Jian Cai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 29 13:01:18 PDT 2019


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


================
Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:52
+
+void e() {
+  int pipefd[2];
----------------
srhines wrote:
> jcai19 wrote:
> > srhines wrote:
> > > I'm not all that familiar with writing clang-tidy-specific tests, but should these tests here denote that a diagnostic should NOT be issued? That is usually the convention in regular Clang tests, so I assume the test runner here should be equally supportive of ensuring that the contents passed through without any other diagnostics related to pipe2 and/or O_CLOEXEC.
> > That makes sense, and I have seem tests for similar checks with (e.g. android-cloexec-open)  and without (e.g. android-cloexec-accep4 and android-cloexec-socket) additional CHECK-MESSAGES-NOT check. But based on the Testing Checks section of https://clang.llvm.org/extra/clang-tidy/Contributing.html, it seems typically CHECK-MASSAGES and CHECK-FIXES are sufficient for clang-tidy checks. Please let me know what you think.
> If you look in test/clang-tidy/android-cloexec-creat.cpp, you will see that there are "CHECK-MESSAGES-NOT" checks that ensure the diagnostic is not issued in correct cases. You can put the checks on lines 39, 58, and 67, which will ensure that there are no additional diagnostics being generated.
Sounds good! Thanks for the reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049





More information about the cfe-commits mailing list