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

Thu May 23 11:06:52 PDT 2019

srhines added inline comments.

================
Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:52
+
+void e() {
+  int pipefd[2];
----------------
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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049