From d8f26e2ac94fe30def727b37e55547c3aa77bbe3 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 13 Sep 2016 10:44:46 -0700 Subject: [PATCH] Base: Fix dangling-else in CHECK_STROP Follow-up to commit 2527628edae5651cb28e72b2c98f24e72dcdb384. Bug: 26962895 Bug: 31338270 Test: m Test: mmma system/core/base && $ANDROID_HOST_OUT/nativetest64/libbase_test/libbase_test64 Change-Id: Ifd71314e146ebf3957cc053ee95ef85002c909b4 --- base/include/android-base/logging.h | 15 +++++++-------- base/logging_test.cpp | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/base/include/android-base/logging.h b/base/include/android-base/logging.h index 39f468977..e8b445f26 100644 --- a/base/include/android-base/logging.h +++ b/base/include/android-base/logging.h @@ -201,14 +201,13 @@ class ErrnoRestorer { #define CHECK_GT(x, y) CHECK_OP(x, y, > ) // Helper for CHECK_STRxx(s1,s2) macros. -#define CHECK_STROP(s1, s2, sense) \ - if (LIKELY((strcmp(s1, s2) == 0) == (sense))) \ - ; \ - else \ - ABORT_AFTER_LOG_FATAL \ - LOG(FATAL) << "Check failed: " \ - << "\"" << (s1) << "\"" \ - << ((sense) ? " == " : " != ") << "\"" << (s2) << "\"" +#define CHECK_STROP(s1, s2, sense) \ + while (UNLIKELY((strcmp(s1, s2) == 0) != (sense))) \ + ABORT_AFTER_LOG_FATAL \ + ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::DEFAULT, \ + ::android::base::FATAL, -1).stream() \ + << "Check failed: " << "\"" << (s1) << "\"" \ + << ((sense) ? " == " : " != ") << "\"" << (s2) << "\"" // Check for string (const char*) equality between s1 and s2, LOG(FATAL) if not. #define CHECK_STREQ(s1, s2) CHECK_STROP(s1, s2, true) diff --git a/base/logging_test.cpp b/base/logging_test.cpp index 02a919870..881352545 100644 --- a/base/logging_test.cpp +++ b/base/logging_test.cpp @@ -120,6 +120,28 @@ TEST(logging, CHECK) { EXPECT_FALSE(flag) << "CHECK_STREQ probably has a dangling if with no else"; } +TEST(logging, DCHECK) { + if (android::base::kEnableDChecks) { + ASSERT_DEATH({SuppressAbortUI(); DCHECK(false);}, "DCheck failed: false "); + } + DCHECK(true); + + if (android::base::kEnableDChecks) { + ASSERT_DEATH({SuppressAbortUI(); DCHECK_EQ(0, 1);}, "DCheck failed: 0 == 1 "); + } + DCHECK_EQ(0, 0); + + if (android::base::kEnableDChecks) { + ASSERT_DEATH({SuppressAbortUI(); DCHECK_STREQ("foo", "bar");}, + R"(DCheck failed: "foo" == "bar")"); + } + DCHECK_STREQ("foo", "foo"); + + // No testing whether we have a dangling else, possibly. That's inherent to the if (constexpr) + // setup we intentionally chose to force type-checks of debug code even in release builds (so + // we don't get more bit-rot). +} + static std::string make_log_pattern(android::base::LogSeverity severity, const char* message) { static const char log_characters[] = "VDIWEFF";