From c6ad69d1d467580455aacc2399e155610f76def6 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 25 Mar 2019 14:55:39 -0700 Subject: [PATCH] liblog: don't return 0xFFFFFFFF as an invalid log id. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are a bunch of branches that check "id >= LOG_ID_MAX", but because C++ hates you, this does a promotion to signed int despite the fact that both sides of the comparison are the same enum with an underlying type of unsigned int. (C++17 ยง7.6.3) Return LOG_ID_MAX instead of a value that gets promoted to signed -1, to avoid this. Bug: http://b/129272512 Test: /data/nativetest64/logcat-unit-tests/logcat-unit-tests Change-Id: I4b3ee662d76d5cc80d9a9625d17f7e5b5980de41 --- liblog/logger_name.cpp | 6 ++++-- logcat/tests/logcat_test.cpp | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/liblog/logger_name.cpp b/liblog/logger_name.cpp index 3aa684132..ece05505b 100644 --- a/liblog/logger_name.cpp +++ b/liblog/logger_name.cpp @@ -50,8 +50,9 @@ log_id_t android_name_to_log_id(const char* logName) { unsigned int ret; if (!logName) { - return static_cast(0xFFFFFFFF); + return static_cast(LOG_ID_MAX); } + b = strrchr(logName, '/'); if (!b) { b = logName; @@ -65,5 +66,6 @@ log_id_t android_name_to_log_id(const char* logName) { return static_cast(ret); } } - return static_cast(0xFFFFFFFF); /* should never happen */ + + return static_cast(LOG_ID_MAX); } diff --git a/logcat/tests/logcat_test.cpp b/logcat/tests/logcat_test.cpp index d5c40bec0..b32b43737 100644 --- a/logcat/tests/logcat_test.cpp +++ b/logcat/tests/logcat_test.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -1747,3 +1748,13 @@ TEST(logcat, help) { EXPECT_EQ(logcatHelpTextSize * 2, logcatLastHelpTextSize); #endif } + +TEST(logcat, invalid_buffer) { + FILE* fp = popen("logcat -b foo 2>&1", "r"); + ASSERT_NE(nullptr, fp); + std::string output; + ASSERT_TRUE(android::base::ReadFdToString(fileno(fp), &output)); + pclose(fp); + + ASSERT_TRUE(android::base::StartsWith(output, "unknown buffer foo\n")); +}