[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() 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/test/clang-tidy/android-cloexec-pipe2.cpp:20
+  pipe2(pipefd, O_NONBLOCK);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC where possible [android-cloexec-pipe2]
+  // CHECK-FIXES: pipe2(pipefd, O_NONBLOCK | O_CLOEXEC);
----------------
gribozavr wrote:
> Same comment about the message as in D61967 -- the message should briefly explain why the user should make this change. Users tend to ignore warnings they don't understand.
> 
> "pipe2 should be called with O_CLOEXEC to avoid leaking file descriptors to child processes"
It appeared to me that there was no easy way to change the warning message due to the way insertMacroFlag is implemented (called in  CloexecPipe2Check::check(...) to add O_CLOEXEC flag when necessary). The function always issue a warning in the format of "%0 should use %1 where possible". So unless we parameterize the warning string (which could be solved by a different code change), we might end up implementing a similar function with different warning message, which creates extra work if we need to modify insertMacroFlag in the future. I am new to Clang so please let me know if there is a better way to udpate the warning message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62049