yawanng created this revision.
yawanng added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, JDevlieghere.
Handle a case when the function is passed as an argument of a function-like
macro. Plus fix a format that was forgotten to commit last time.
Repository:
rL LLVM
https://reviews.llvm.org/D34633
Files:
clang-tidy/android/FileOpenFlagCheck.cpp
test/clang-tidy/android-file-open-flag.cpp
Index: test/clang-tidy/android-file-open-flag.cpp
===================================================================
--- test/clang-tidy/android-file-open-flag.cpp
+++ test/clang-tidy/android-file-open-flag.cpp
@@ -4,6 +4,13 @@
#define O_EXCL 2
#define __O_CLOEXEC 3
#define O_CLOEXEC __O_CLOEXEC
+#define TEMP_FAILURE_RETRY(exp) \
+ ({ \
+ int _rc; \
+ do { \
+ _rc = (exp); \
+ } while (_rc == -1); \
+ })
extern "C" int open(const char *fn, int flags, ...);
extern "C" int open64(const char *fn, int flags, ...);
@@ -13,47 +20,80 @@
open("filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
+ TEMP_FAILURE_RETRY(open("filename", O_RDWR));
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'open' should use O_CLOEXEC where
+ // CHECK-FIXES: O_RDWR | O_CLOEXEC
open("filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: 'open' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
+ TEMP_FAILURE_RETRY(open("filename", O_RDWR | O_EXCL));
+ // CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'open' should use O_CLOEXEC where
+ // CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void b() {
open64("filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
+ TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
+ // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: 'open64' should use O_CLOEXEC where
+ // CHECK-FIXES: O_RDWR | O_CLOEXEC
open64("filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: 'open64' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
+ TEMP_FAILURE_RETRY(open64("filename", O_RDWR | O_EXCL));
+ // CHECK-MESSAGES: :[[@LINE-1]]:56: warning: 'open64' should use O_CLOEXEC where
+ // CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void c() {
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: O_RDWR | O_CLOEXEC
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
+ // CHECK-MESSAGES: :[[@LINE-1]]:50: warning: 'openat' should use O_CLOEXEC where
+ // CHECK-FIXES: O_RDWR | O_CLOEXEC
openat(0, "filename", O_RDWR | O_EXCL);
// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'openat' should use O_CLOEXEC where
// CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR | O_EXCL));
+ // CHECK-MESSAGES: :[[@LINE-1]]:59: warning: 'openat' should use O_CLOEXEC where
+ // CHECK-FIXES: O_RDWR | O_EXCL | O_CLOEXEC
}
void f() {
open("filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'open' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: 3 | O_CLOEXEC
+ TEMP_FAILURE_RETRY(open("filename", 3));
+ // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: 'open' should use O_CLOEXEC where
+ // CHECK-FIXES: 3 | O_CLOEXEC
open64("filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: 'open64' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: 3 | O_CLOEXEC
+ TEMP_FAILURE_RETRY(open64("filename", 3));
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: 'open64' should use O_CLOEXEC where
+ // CHECK-FIXES: 3 | O_CLOEXEC
openat(0, "filename", 3);
// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: 'openat' should use O_CLOEXEC where possible [android-file-open-flag]
// CHECK-FIXES: 3 | O_CLOEXEC
+ TEMP_FAILURE_RETRY(openat(0, "filename", 3));
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: 'openat' should use O_CLOEXEC where
+ // CHECK-FIXES: 3 | O_CLOEXEC
int flag = 3;
open("filename", flag);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open("filename", flag));
+ // CHECK-MESSAGES-NOT: warning:
open64("filename", flag);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open64("filename", flag));
+ // CHECK-MESSAGES-NOT: warning:
openat(0, "filename", flag);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(openat(0, "filename", flag));
+ // CHECK-MESSAGES-NOT: warning:
}
namespace i {
@@ -64,10 +104,16 @@
void d() {
open("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open("filename", O_RDWR));
+ // CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
+ // CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
+ // CHECK-MESSAGES-NOT: warning:
}
} // namespace i
@@ -75,22 +121,40 @@
void e() {
open("filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open("filename", O_CLOEXEC));
+ // CHECK-MESSAGES-NOT: warning:
open("filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open("filename", O_RDWR | O_CLOEXEC));
+ // CHECK-MESSAGES-NOT: warning:
open("filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open("filename", O_RDWR | O_CLOEXEC | O_EXCL));
+ // CHECK-MESSAGES-NOT: warning:
open64("filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open64("filename", O_CLOEXEC));
+ // CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open64("filename", O_RDWR | O_CLOEXEC));
+ // CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open64("filename", O_RDWR | O_CLOEXEC | O_EXCL));
+ // CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_CLOEXEC));
+ // CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR | O_CLOEXEC);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR | O_CLOEXEC));
+ // CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR | O_CLOEXEC | O_EXCL);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR | O_CLOEXEC | O_EXCL));
+ // CHECK-MESSAGES-NOT: warning:
}
class G {
@@ -102,9 +166,15 @@
void h() {
open("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open("filename", O_RDWR));
+ // CHECK-MESSAGES-NOT: warning:
open64("filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(open64("filename", O_RDWR));
+ // CHECK-MESSAGES-NOT: warning:
openat(0, "filename", O_RDWR);
// CHECK-MESSAGES-NOT: warning:
+ TEMP_FAILURE_RETRY(openat(0, "filename", O_RDWR));
+ // CHECK-MESSAGES-NOT: warning:
}
};
Index: clang-tidy/android/FileOpenFlagCheck.cpp
===================================================================
--- clang-tidy/android/FileOpenFlagCheck.cpp
+++ clang-tidy/android/FileOpenFlagCheck.cpp
@@ -21,11 +21,12 @@
namespace {
static constexpr const char *O_CLOEXEC = "O_CLOEXEC";
-bool HasCloseOnExecFlag(const Expr *Flags, const SourceManager &SM,
+bool hasCloseOnExecFlag(const Expr *Flags, const SourceManager &SM,
const LangOptions &LangOpts) {
// If the Flag is an integer constant, check it.
if (isa<IntegerLiteral>(Flags)) {
- if (!SM.isMacroBodyExpansion(Flags->getLocStart()))
+ if (!SM.isMacroBodyExpansion(Flags->getLocStart()) &&
+ !SM.isMacroArgExpansion(Flags->getLocStart()))
return false;
// Get the Marco name.
@@ -37,9 +38,9 @@
// If it's a binary OR operation.
if (const auto *BO = dyn_cast<BinaryOperator>(Flags))
if (BO->getOpcode() == clang::BinaryOperatorKind::BO_Or)
- return HasCloseOnExecFlag(BO->getLHS()->IgnoreParenCasts(), SM,
+ return hasCloseOnExecFlag(BO->getLHS()->IgnoreParenCasts(), SM,
LangOpts) ||
- HasCloseOnExecFlag(BO->getRHS()->IgnoreParenCasts(), SM, LangOpts);
+ hasCloseOnExecFlag(BO->getRHS()->IgnoreParenCasts(), SM, LangOpts);
// Otherwise, assume it has the flag.
return true;
@@ -81,12 +82,13 @@
// Check the required flag.
SourceManager &SM = *Result.SourceManager;
- if (HasCloseOnExecFlag(FlagArg->IgnoreParenCasts(), SM,
+ if (hasCloseOnExecFlag(FlagArg->IgnoreParenCasts(), SM,
Result.Context->getLangOpts()))
return;
- SourceLocation EndLoc = Lexer::getLocForEndOfToken(
- FlagArg->getLocEnd(), 0, SM, Result.Context->getLangOpts());
+ SourceLocation EndLoc =
+ Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
+ Result.Context->getLangOpts());
diag(EndLoc, "%0 should use %1 where possible")
<< FD << O_CLOEXEC
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
