From 2ec6a53a46b111f67569fa7056701235a6353f3a Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 27 Jan 2020 08:35:13 -0800 Subject: [PATCH] liblog: use default tag for loggability checks if no tag is provided Bug: 116329414 Bug: 119867234 Test: new unit tests Change-Id: I92a3f4f95e5f482f6fe20f17ed83c8ed367b06dc --- base/logging.cpp | 1 - liblog/logger_write.cpp | 6 ++-- liblog/logger_write.h | 24 ++++++++++++++++ liblog/properties.cpp | 18 +++++++++--- liblog/tests/liblog_default_tag.cpp | 44 +++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 liblog/logger_write.h diff --git a/base/logging.cpp b/base/logging.cpp index 1d8ef573d..a8687067a 100644 --- a/base/logging.cpp +++ b/base/logging.cpp @@ -608,7 +608,6 @@ bool ShouldLog(LogSeverity severity, const char* tag) { // we need to fall back to using gMinimumLogSeverity, since __android_log_is_loggable() will not // take into consideration the value from SetMinimumLogSeverity(). if (liblog_functions) { - // TODO: It is safe to pass nullptr for tag, but it will be better to use the default log tag. int priority = LogSeverityToPriority(severity); return __android_log_is_loggable(priority, tag, ANDROID_LOG_INFO); } else { diff --git a/liblog/logger_write.cpp b/liblog/logger_write.cpp index e86d9ec13..37333570d 100644 --- a/liblog/logger_write.cpp +++ b/liblog/logger_write.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "logger_write.h" + #include #include #include @@ -136,11 +138,11 @@ static const char* getprogname() { // It's possible for logging to happen during static initialization before our globals are // initialized, so we place this std::string in a function such that it is initialized on the first // call. -static std::string& GetDefaultTag() { +std::string& GetDefaultTag() { static std::string default_tag = getprogname(); return default_tag; } -static RwLock default_tag_lock; +RwLock default_tag_lock; void __android_log_set_default_tag(const char* tag) { auto lock = std::unique_lock{default_tag_lock}; diff --git a/liblog/logger_write.h b/liblog/logger_write.h new file mode 100644 index 000000000..065fd55fd --- /dev/null +++ b/liblog/logger_write.h @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include "rwlock.h" + +std::string& GetDefaultTag(); // Must read lock default_tag_lock +extern RwLock default_tag_lock; \ No newline at end of file diff --git a/liblog/properties.cpp b/liblog/properties.cpp index a53c92b81..b13662fa0 100644 --- a/liblog/properties.cpp +++ b/liblog/properties.cpp @@ -25,9 +25,12 @@ #include #include +#include #include +#include "logger_write.h" + static pthread_mutex_t lock_loggable = PTHREAD_MUTEX_INITIALIZER; static int lock() { @@ -93,10 +96,17 @@ static int __android_log_level(const char* tag, size_t len) { /* sizeof() is used on this array below */ static const char log_namespace[] = "persist.log.tag."; static const size_t base_offset = 8; /* skip "persist." */ - /* calculate the size of our key temporary buffer */ - const size_t taglen = tag ? len : 0; + + auto tag_lock = std::shared_lock{default_tag_lock, std::defer_lock}; + if (tag == nullptr || len == 0) { + tag_lock.lock(); + auto& tag_string = GetDefaultTag(); + tag = tag_string.c_str(); + len = tag_string.size(); + } + /* sizeof(log_namespace) = strlen(log_namespace) + 1 */ - char key[sizeof(log_namespace) + taglen]; + char key[sizeof(log_namespace) + len]; char* kp; size_t i; char c = 0; @@ -146,7 +156,7 @@ static int __android_log_level(const char* tag, size_t len) { } } - if (taglen) { + if (len) { int local_change_detected = change_detected; if (!not_locked) { if (!last_tag || !last_tag[0] || (last_tag[0] != tag[0]) || diff --git a/liblog/tests/liblog_default_tag.cpp b/liblog/tests/liblog_default_tag.cpp index a5baa9ff9..5643f6377 100644 --- a/liblog/tests/liblog_default_tag.cpp +++ b/liblog/tests/liblog_default_tag.cpp @@ -96,4 +96,48 @@ TEST(liblog_default_tag, liblog_sets_default_tag) { LOG(WARNING) << "message"; EXPECT_TRUE(message_seen); +} + +TEST(liblog_default_tag, default_tag_plus_log_severity) { + using namespace android::base; + bool message_seen = false; + std::string expected_tag = "liblog_test_tag"; + SetLogger([&](LogId, LogSeverity, const char* tag, const char*, unsigned int, const char*) { + message_seen = true; + EXPECT_EQ(expected_tag, tag); + }); + __android_log_set_default_tag(expected_tag.c_str()); + + auto log_tag_property = "log.tag." + expected_tag; + SetProperty(log_tag_property, "V"); + + __android_log_buf_write(LOG_ID_MAIN, ANDROID_LOG_VERBOSE, nullptr, "message"); + EXPECT_TRUE(message_seen); + message_seen = false; + + LOG(VERBOSE) << "message"; + EXPECT_TRUE(message_seen); +} + +TEST(liblog_default_tag, generated_default_tag_plus_log_severity) { + using namespace android::base; + bool message_seen = false; + std::string expected_tag = getprogname(); + SetLogger([&](LogId, LogSeverity, const char* tag, const char*, unsigned int, const char*) { + message_seen = true; + EXPECT_EQ(expected_tag, tag); + }); + + // Even without any calls to SetDefaultTag(), the first message that attempts to log, will + // generate a default tag from getprogname() and check log.tag. for loggability. This + // case checks that we can log a Verbose message when log.tag. is set to 'V'. + auto log_tag_property = "log.tag." + expected_tag; + SetProperty(log_tag_property, "V"); + + __android_log_buf_write(LOG_ID_MAIN, ANDROID_LOG_VERBOSE, nullptr, "message"); + EXPECT_TRUE(message_seen); + message_seen = false; + + LOG(VERBOSE) << "message"; + EXPECT_TRUE(message_seen); } \ No newline at end of file