From d11f2371e135efc4a7fbae948f41dd843b6d4df8 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 14 Aug 2020 09:35:16 -0700 Subject: [PATCH] Remove android_lookupEventTagNum() and related code This was introduced years ago but never gained any users. Test: build Merged-In: Id2deb6de1839f995970c6350a1970a872f0b51cf Change-Id: Id2deb6de1839f995970c6350a1970a872f0b51cf --- liblog/event_tag_map.cpp | 287 ++--------------------------- liblog/include/log/event_tag_map.h | 6 - liblog/liblog.map.txt | 1 - liblog/tests/liblog_benchmark.cpp | 24 --- liblog/tests/liblog_test.cpp | 17 -- logcat/tests/logcat_test.cpp | 42 ----- 6 files changed, 19 insertions(+), 358 deletions(-) diff --git a/liblog/event_tag_map.cpp b/liblog/event_tag_map.cpp index 2c0156ea2..85556e86f 100644 --- a/liblog/event_tag_map.cpp +++ b/liblog/event_tag_map.cpp @@ -31,84 +31,13 @@ #include #include -#include #include #include #include -#include "logd_reader.h" - #define OUT_TAG "EventTagMap" -class MapString { - private: - const std::string* alloc; // HAS-AN - const std::string_view str; // HAS-A - - public: - operator const std::string_view() const { - return str; - } - - const char* data() const { - return str.data(); - } - size_t length() const { - return str.length(); - } - - bool operator==(const MapString& rval) const { - if (length() != rval.length()) return false; - if (length() == 0) return true; - return fastcmp(data(), rval.data(), length()) == 0; - } - bool operator!=(const MapString& rval) const { - return !(*this == rval); - } - - MapString(const char* str, size_t len) : alloc(NULL), str(str, len) { - } - explicit MapString(const std::string& str) - : alloc(new std::string(str)), str(alloc->data(), alloc->length()) { - } - MapString(MapString&& rval) noexcept - : alloc(rval.alloc), str(rval.data(), rval.length()) { - rval.alloc = NULL; - } - explicit MapString(const MapString& rval) - : alloc(rval.alloc ? new std::string(*rval.alloc) : NULL), - str(alloc ? alloc->data() : rval.data(), rval.length()) { - } - - ~MapString() { - if (alloc) delete alloc; - } -}; - -// Hash for MapString -template <> -struct std::hash - : public std::unary_function { - size_t operator()(const MapString& __t) const noexcept { - if (!__t.length()) return 0; - return std::hash()(std::string_view(__t)); - } -}; - -typedef std::pair TagFmt; - -template <> -struct std::hash : public std::unary_function { - size_t operator()(const TagFmt& __t) const noexcept { - // Tag is typically unique. Will cost us an extra 100ns for the - // unordered_map lookup if we instead did a hash that combined - // both of tag and fmt members, e.g.: - // - // return std::hash()(__t.first) ^ - // std::hash()(__t.second); - return std::hash()(__t.first); - } -}; +typedef std::pair TagFmt; // Map struct EventTagMap { @@ -119,8 +48,7 @@ struct EventTagMap { private: std::unordered_map Idx2TagFmt; - std::unordered_map TagFmt2Idx; - std::unordered_map Tag2Idx; + std::unordered_map Tag2Idx; // protect unordered sets android::RWLock rwlock; @@ -132,7 +60,6 @@ struct EventTagMap { ~EventTagMap() { Idx2TagFmt.clear(); - TagFmt2Idx.clear(); Tag2Idx.clear(); for (size_t which = 0; which < NUM_MAPS; ++which) { if (mapAddr[which]) { @@ -144,8 +71,7 @@ struct EventTagMap { bool emplaceUnique(uint32_t tag, const TagFmt& tagfmt, bool verbose = false); const TagFmt* find(uint32_t tag) const; - int find(TagFmt&& tagfmt) const; - int find(MapString&& tag) const; + int find(std::string_view tag) const; }; bool EventTagMap::emplaceUnique(uint32_t tag, const TagFmt& tagfmt, @@ -156,8 +82,7 @@ bool EventTagMap::emplaceUnique(uint32_t tag, const TagFmt& tagfmt, ":%.*s:%.*s)\n"; android::RWLock::AutoWLock writeLock(rwlock); { - std::unordered_map::const_iterator it; - it = Idx2TagFmt.find(tag); + auto it = Idx2TagFmt.find(tag); if (it != Idx2TagFmt.end()) { if (verbose) { fprintf(stderr, errorFormat, it->first, (int)it->second.first.length(), @@ -173,25 +98,7 @@ bool EventTagMap::emplaceUnique(uint32_t tag, const TagFmt& tagfmt, } { - std::unordered_map::const_iterator it; - it = TagFmt2Idx.find(tagfmt); - if (it != TagFmt2Idx.end()) { - if (verbose) { - fprintf(stderr, errorFormat, it->second, (int)it->first.first.length(), - it->first.first.data(), (int)it->first.second.length(), - it->first.second.data(), tag, (int)tagfmt.first.length(), - tagfmt.first.data(), (int)tagfmt.second.length(), - tagfmt.second.data()); - } - ret = false; - } else { - TagFmt2Idx.emplace(std::make_pair(tagfmt, tag)); - } - } - - { - std::unordered_map::const_iterator it; - it = Tag2Idx.find(tagfmt.first); + auto it = Tag2Idx.find(tagfmt.first); if (!tagfmt.second.length() && (it != Tag2Idx.end())) { Tag2Idx.erase(it); it = Tag2Idx.end(); @@ -205,25 +112,15 @@ bool EventTagMap::emplaceUnique(uint32_t tag, const TagFmt& tagfmt, } const TagFmt* EventTagMap::find(uint32_t tag) const { - std::unordered_map::const_iterator it; android::RWLock::AutoRLock readLock(const_cast(rwlock)); - it = Idx2TagFmt.find(tag); + auto it = Idx2TagFmt.find(tag); if (it == Idx2TagFmt.end()) return NULL; return &(it->second); } -int EventTagMap::find(TagFmt&& tagfmt) const { - std::unordered_map::const_iterator it; +int EventTagMap::find(std::string_view tag) const { android::RWLock::AutoRLock readLock(const_cast(rwlock)); - it = TagFmt2Idx.find(std::move(tagfmt)); - if (it == TagFmt2Idx.end()) return -1; - return it->second; -} - -int EventTagMap::find(MapString&& tag) const { - std::unordered_map::const_iterator it; - android::RWLock::AutoRLock readLock(const_cast(rwlock)); - it = Tag2Idx.find(std::move(tag)); + auto it = Tag2Idx.find(std::move(tag)); if (it == Tag2Idx.end()) return -1; return it->second; } @@ -241,29 +138,20 @@ static const char* endOfTag(const char* cp) { // successful return, it will be pointing to the last character in the // tag line (i.e. the character before the start of the next line). // -// lineNum = 0 removes verbose comments and requires us to cache the -// content rather than make direct raw references since the content -// will disappear after the call. A non-zero lineNum means we own the -// data and it will outlive the call. -// // Returns 0 on success, nonzero on failure. -static int scanTagLine(EventTagMap* map, const char*& pData, int lineNum) { +static int scanTagLine(EventTagMap* map, const char*& pData, int line_num) { char* ep; unsigned long val = strtoul(pData, &ep, 10); const char* cp = ep; if (cp == pData) { - if (lineNum) { - fprintf(stderr, OUT_TAG ": malformed tag number on line %d\n", lineNum); - } + fprintf(stderr, OUT_TAG ": malformed tag number on line %d\n", line_num); errno = EINVAL; return -1; } uint32_t tagIndex = val; if (tagIndex != val) { - if (lineNum) { - fprintf(stderr, OUT_TAG ": tag number too large on line %d\n", lineNum); - } + fprintf(stderr, OUT_TAG ": tag number too large on line %d\n", line_num); errno = ERANGE; return -1; } @@ -272,9 +160,7 @@ static int scanTagLine(EventTagMap* map, const char*& pData, int lineNum) { } if (*cp == '\n') { - if (lineNum) { - fprintf(stderr, OUT_TAG ": missing tag string on line %d\n", lineNum); - } + fprintf(stderr, OUT_TAG ": missing tag string on line %d\n", line_num); errno = EINVAL; return -1; } @@ -284,10 +170,7 @@ static int scanTagLine(EventTagMap* map, const char*& pData, int lineNum) { size_t tagLen = cp - tag; if (!isspace(*cp)) { - if (lineNum) { - fprintf(stderr, OUT_TAG ": invalid tag char %c on line %d\n", *cp, - lineNum); - } + fprintf(stderr, OUT_TAG ": invalid tag char %c on line %d\n", *cp, line_num); errno = EINVAL; return -1; } @@ -317,25 +200,15 @@ static int scanTagLine(EventTagMap* map, const char*& pData, int lineNum) { while (*cp && (*cp != '\n')) ++cp; #ifdef DEBUG - fprintf(stderr, "%d: %p: %.*s\n", lineNum, tag, (int)(cp - pData), pData); + fprintf(stderr, "%d: %p: %.*s\n", line_num, tag, (int)(cp - pData), pData); #endif pData = cp; - if (lineNum) { - if (map->emplaceUnique(tagIndex, - TagFmt(std::make_pair(MapString(tag, tagLen), - MapString(fmt, fmtLen))), - verbose)) { - return 0; - } - } else { - // cache - if (map->emplaceUnique( - tagIndex, - TagFmt(std::make_pair(MapString(std::string(tag, tagLen)), - MapString(std::string(fmt, fmtLen)))))) { - return 0; - } + if (map->emplaceUnique( + tagIndex, + TagFmt(std::make_pair(std::string_view(tag, tagLen), std::string_view(fmt, fmtLen))), + verbose)) { + return 0; } errno = EMLINK; return -1; @@ -491,57 +364,10 @@ void android_closeEventTagMap(EventTagMap* map) { if (map) delete map; } -// Cache miss, go to logd to acquire a public reference. -// Because we lack access to a SHARED PUBLIC /dev/event-log-tags file map? -static const TagFmt* __getEventTag([[maybe_unused]] EventTagMap* map, unsigned int tag) { - // call event tag service to arrange for a new tag - char* buf = NULL; - // Can not use android::base::StringPrintf, asprintf + free instead. - static const char command_template[] = "getEventTag id=%u"; - int ret = asprintf(&buf, command_template, tag); - if (ret > 0) { - // Add some buffer margin for an estimate of the full return content. - size_t size = - ret - strlen(command_template) + - strlen("65535\n4294967295\t?\t\t\t?\t# uid=32767\n\n\f?success?"); - if (size > (size_t)ret) { - char* np = static_cast(realloc(buf, size)); - if (np) { - buf = np; - } else { - size = ret; - } - } else { - size = ret; - } -#ifdef __ANDROID__ - // Ask event log tag service for an existing entry - if (SendLogdControlMessage(buf, size) >= 0) { - buf[size - 1] = '\0'; - char* ep; - unsigned long val = strtoul(buf, &ep, 10); // return size - const char* cp = ep; - if ((buf != cp) && (val > 0) && (*cp == '\n')) { // truncation OK - ++cp; - if (!scanTagLine(map, cp, 0)) { - free(buf); - return map->find(tag); - } - } - } -#endif - free(buf); - } - return NULL; -} - // Look up an entry in the map. const char* android_lookupEventTag_len(const EventTagMap* map, size_t* len, unsigned int tag) { if (len) *len = 0; const TagFmt* str = map->find(tag); - if (!str) { - str = __getEventTag(const_cast(map), tag); - } if (!str) return NULL; if (len) *len = str->first.length(); return str->first.data(); @@ -551,83 +377,8 @@ const char* android_lookupEventTag_len(const EventTagMap* map, size_t* len, unsi const char* android_lookupEventFormat_len(const EventTagMap* map, size_t* len, unsigned int tag) { if (len) *len = 0; const TagFmt* str = map->find(tag); - if (!str) { - str = __getEventTag(const_cast(map), tag); - } if (!str) return NULL; if (len) *len = str->second.length(); return str->second.data(); } -// Look up tagname, generate one if necessary, and return a tag -int android_lookupEventTagNum(EventTagMap* map, const char* tagname, const char* format, int prio) { - const char* ep = endOfTag(tagname); - size_t len = ep - tagname; - if (!len || *ep) { - errno = EINVAL; - return -1; - } - - if ((prio != ANDROID_LOG_UNKNOWN) && (prio < ANDROID_LOG_SILENT) && - !__android_log_is_loggable_len(prio, tagname, len, - __android_log_is_debuggable() - ? ANDROID_LOG_VERBOSE - : ANDROID_LOG_DEBUG)) { - errno = EPERM; - return -1; - } - - if (!format) format = ""; - ssize_t fmtLen = strlen(format); - int ret = map->find(TagFmt( - std::make_pair(MapString(tagname, len), MapString(format, fmtLen)))); - if (ret != -1) return ret; - - // call event tag service to arrange for a new tag - char* buf = NULL; - // Can not use android::base::StringPrintf, asprintf + free instead. - static const char command_template[] = "getEventTag name=%s format=\"%s\""; - ret = asprintf(&buf, command_template, tagname, format); - if (ret > 0) { - // Add some buffer margin for an estimate of the full return content. - char* cp; - size_t size = - ret - strlen(command_template) + - strlen("65535\n4294967295\t?\t\t\t?\t# uid=32767\n\n\f?success?"); - if (size > (size_t)ret) { - cp = static_cast(realloc(buf, size)); - if (cp) { - buf = cp; - } else { - size = ret; - } - } else { - size = ret; - } -#ifdef __ANDROID__ - // Ask event log tag service for an allocation - if (SendLogdControlMessage(buf, size) >= 0) { - buf[size - 1] = '\0'; - unsigned long val = strtoul(buf, &cp, 10); // return size - if ((buf != cp) && (val > 0) && (*cp == '\n')) { // truncation OK - val = strtoul(cp + 1, &cp, 10); // allocated tag number - if ((val > 0) && (val < UINT32_MAX) && (*cp == '\t')) { - free(buf); - ret = val; - // cache - map->emplaceUnique(ret, TagFmt(std::make_pair( - MapString(std::string(tagname, len)), - MapString(std::string(format, fmtLen))))); - return ret; - } - } - } -#endif - free(buf); - } - - // Hail Mary - ret = map->find(MapString(tagname, len)); - if (ret == -1) errno = ESRCH; - return ret; -} diff --git a/liblog/include/log/event_tag_map.h b/liblog/include/log/event_tag_map.h index 4d0ebf954..de49fbf9e 100644 --- a/liblog/include/log/event_tag_map.h +++ b/liblog/include/log/event_tag_map.h @@ -53,12 +53,6 @@ const char* android_lookupEventTag_len(const EventTagMap* map, size_t* len, const char* android_lookupEventFormat_len(const EventTagMap* map, size_t* len, unsigned int tag); -/* - * Look up tagname, generate one if necessary, and return a tag - */ -int android_lookupEventTagNum(EventTagMap* map, const char* tagname, - const char* format, int prio); - #ifdef __cplusplus } #endif diff --git a/liblog/liblog.map.txt b/liblog/liblog.map.txt index 8beb679c3..f8d5ef0f3 100644 --- a/liblog/liblog.map.txt +++ b/liblog/liblog.map.txt @@ -89,6 +89,5 @@ LIBLOG_PRIVATE { android_log_processLogBuffer; android_log_read_next; android_log_write_list_buffer; - android_lookupEventTagNum; create_android_log_parser; }; diff --git a/liblog/tests/liblog_benchmark.cpp b/liblog/tests/liblog_benchmark.cpp index 3bd5cf20f..d2f12d6d2 100644 --- a/liblog/tests/liblog_benchmark.cpp +++ b/liblog/tests/liblog_benchmark.cpp @@ -879,30 +879,6 @@ static void BM_lookupEventFormat(benchmark::State& state) { } BENCHMARK(BM_lookupEventFormat); -/* - * Measure the time it takes for android_lookupEventTagNum plus above - */ -static void BM_lookupEventTagNum(benchmark::State& state) { - prechargeEventMap(); - - std::unordered_set::const_iterator it = set.begin(); - - while (state.KeepRunning()) { - size_t len; - const char* name = android_lookupEventTag_len(map, &len, (*it)); - std::string Name(name, len); - const char* format = android_lookupEventFormat_len(map, &len, (*it)); - std::string Format(format, len); - state.ResumeTiming(); - android_lookupEventTagNum(map, Name.c_str(), Format.c_str(), - ANDROID_LOG_UNKNOWN); - state.PauseTiming(); - ++it; - if (it == set.end()) it = set.begin(); - } -} -BENCHMARK(BM_lookupEventTagNum); - // Must be functionally identical to liblog internal SendLogdControlMessage() static void send_to_control(char* buf, size_t len) { int sock = diff --git a/liblog/tests/liblog_test.cpp b/liblog/tests/liblog_test.cpp index fbc3d7a5a..c49d87b59 100644 --- a/liblog/tests/liblog_test.cpp +++ b/liblog/tests/liblog_test.cpp @@ -2768,20 +2768,3 @@ TEST(liblog, __android_log_pmsg_file_read) { #endif } #endif // ENABLE_FLAKY_TESTS - -TEST(liblog, android_lookupEventTagNum) { -#ifdef __ANDROID__ - EventTagMap* map = android_openEventTagMap(NULL); - EXPECT_TRUE(NULL != map); - std::string Name = android::base::StringPrintf("a%d", getpid()); - int tag = android_lookupEventTagNum(map, Name.c_str(), "(new|1)", - ANDROID_LOG_UNKNOWN); - android_closeEventTagMap(map); - if (tag == -1) system("tail -3 /dev/event-log-tags >&2"); - EXPECT_NE(-1, tag); - EXPECT_NE(0, tag); - EXPECT_GT(UINT32_MAX, (unsigned)tag); -#else - GTEST_LOG_(INFO) << "This test does nothing.\n"; -#endif -} diff --git a/logcat/tests/logcat_test.cpp b/logcat/tests/logcat_test.cpp index 61aa9381c..08600000d 100644 --- a/logcat/tests/logcat_test.cpp +++ b/logcat/tests/logcat_test.cpp @@ -1654,48 +1654,6 @@ TEST(logcat, descriptive) { EXPECT_GE(ret, 0); EXPECT_TRUE(End_to_End(sync.tagStr, "")); } - - { - // Invent new entries because existing can not serve - EventTagMap* map = android_openEventTagMap(nullptr); - ASSERT_TRUE(nullptr != map); - static const char name[] = logcat_executable ".descriptive-monotonic"; - int myTag = android_lookupEventTagNum(map, name, "(new|1|s)", - ANDROID_LOG_UNKNOWN); - android_closeEventTagMap(map); - ASSERT_NE(-1, myTag); - - const struct tag sync = { (uint32_t)myTag, name }; - - { - android_log_event_list ctx(sync.tagNo); - ctx << (uint32_t)7; - for (ret = -EBUSY; ret == -EBUSY; rest()) ret = ctx.write(); - EXPECT_GE(ret, 0); - EXPECT_TRUE(End_to_End(sync.tagStr, "new=7s")); - } - { - android_log_event_list ctx(sync.tagNo); - ctx << (uint32_t)62; - for (ret = -EBUSY; ret == -EBUSY; rest()) ret = ctx.write(); - EXPECT_GE(ret, 0); - EXPECT_TRUE(End_to_End(sync.tagStr, "new=1:02")); - } - { - android_log_event_list ctx(sync.tagNo); - ctx << (uint32_t)3673; - for (ret = -EBUSY; ret == -EBUSY; rest()) ret = ctx.write(); - EXPECT_GE(ret, 0); - EXPECT_TRUE(End_to_End(sync.tagStr, "new=1:01:13")); - } - { - android_log_event_list ctx(sync.tagNo); - ctx << (uint32_t)(86400 + 7200 + 180 + 58); - for (ret = -EBUSY; ret == -EBUSY; rest()) ret = ctx.write(); - EXPECT_GE(ret, 0); - EXPECT_TRUE(End_to_End(sync.tagStr, "new=1d 2:03:58")); - } - } } static bool reportedSecurity(const char* command) {