From c637ada7e404b4146fb1266947e1072160ca32ae Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris@google.com>
Date: Fri, 13 Jul 2018 16:55:38 -0700
Subject: [PATCH] Clean up logging code.

Test: Verify that tombstones still contain the log message data.
Change-Id: I303dec944e58a0c4f5edfed22caaf1f2462ea3e8
---
 debuggerd/libdebuggerd/tombstone.cpp | 46 ++++++++++++----------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp
index 433bb4657..0b8a9362b 100644
--- a/debuggerd/libdebuggerd/tombstone.cpp
+++ b/debuggerd/libdebuggerd/tombstone.cpp
@@ -474,7 +474,7 @@ static EventTagMap* g_eventTagMap = NULL;
 
 static void dump_log_file(log_t* log, pid_t pid, const char* filename, unsigned int tail) {
   bool first = true;
-  struct logger_list* logger_list;
+  logger_list* logger_list;
 
   if (!log->should_retrieve_logcat) {
     return;
@@ -488,11 +488,9 @@ static void dump_log_file(log_t* log, pid_t pid, const char* filename, unsigned
     return;
   }
 
-  struct log_msg log_entry;
-
   while (true) {
+    log_msg log_entry;
     ssize_t actual = android_logger_list_read(logger_list, &log_entry);
-    struct logger_entry* entry;
 
     if (actual < 0) {
       if (actual == -EINTR) {
@@ -515,8 +513,6 @@ static void dump_log_file(log_t* log, pid_t pid, const char* filename, unsigned
     // high-frequency debug diagnostics should just be written to
     // the tombstone file.
 
-    entry = &log_entry.entry_v1;
-
     if (first) {
       _LOG(log, logtype::LOGS, "--------- %slog %s\n",
         tail ? "tail end of " : "", filename);
@@ -527,19 +523,8 @@ static void dump_log_file(log_t* log, pid_t pid, const char* filename, unsigned
     //
     // We want to display it in the same format as "logcat -v threadtime"
     // (although in this case the pid is redundant).
-    static const char* kPrioChars = "!.VDIWEFS";
-    unsigned hdr_size = log_entry.entry.hdr_size;
-    if (!hdr_size) {
-      hdr_size = sizeof(log_entry.entry_v1);
-    }
-    if ((hdr_size < sizeof(log_entry.entry_v1)) ||
-        (hdr_size > sizeof(log_entry.entry))) {
-      continue;
-    }
-    char* msg = reinterpret_cast<char*>(log_entry.buf) + hdr_size;
-
     char timeBuf[32];
-    time_t sec = static_cast<time_t>(entry->sec);
+    time_t sec = static_cast<time_t>(log_entry.entry.sec);
     struct tm tmBuf;
     struct tm* ptm;
     ptm = localtime_r(&sec, &tmBuf);
@@ -547,17 +532,23 @@ static void dump_log_file(log_t* log, pid_t pid, const char* filename, unsigned
 
     if (log_entry.id() == LOG_ID_EVENTS) {
       if (!g_eventTagMap) {
-        g_eventTagMap = android_openEventTagMap(NULL);
+        g_eventTagMap = android_openEventTagMap(nullptr);
       }
       AndroidLogEntry e;
       char buf[512];
-      android_log_processBinaryLogBuffer(entry, &e, g_eventTagMap, buf, sizeof(buf));
-      _LOG(log, logtype::LOGS, "%s.%03d %5d %5d %c %-8.*s: %s\n",
-         timeBuf, entry->nsec / 1000000, entry->pid, entry->tid,
-         'I', (int)e.tagLen, e.tag, e.message);
+      if (android_log_processBinaryLogBuffer(&log_entry.entry_v1, &e, g_eventTagMap, buf,
+                                             sizeof(buf)) == 0) {
+        _LOG(log, logtype::LOGS, "%s.%03d %5d %5d %c %-8.*s: %s\n", timeBuf,
+             log_entry.entry.nsec / 1000000, log_entry.entry.pid, log_entry.entry.tid, 'I',
+             (int)e.tagLen, e.tag, e.message);
+      }
       continue;
     }
 
+    char* msg = log_entry.msg();
+    if (msg == nullptr) {
+      continue;
+    }
     unsigned char prio = msg[0];
     char* tag = msg + 1;
     msg = tag + strlen(tag) + 1;
@@ -568,20 +559,21 @@ static void dump_log_file(log_t* log, pid_t pid, const char* filename, unsigned
       *nl-- = '\0';
     }
 
+    static const char* kPrioChars = "!.VDIWEFS";
     char prioChar = (prio < strlen(kPrioChars) ? kPrioChars[prio] : '?');
 
     // Look for line breaks ('\n') and display each text line
     // on a separate line, prefixed with the header, like logcat does.
     do {
       nl = strchr(msg, '\n');
-      if (nl) {
+      if (nl != nullptr) {
         *nl = '\0';
         ++nl;
       }
 
-      _LOG(log, logtype::LOGS, "%s.%03d %5d %5d %c %-8s: %s\n",
-         timeBuf, entry->nsec / 1000000, entry->pid, entry->tid,
-         prioChar, tag, msg);
+      _LOG(log, logtype::LOGS, "%s.%03d %5d %5d %c %-8s: %s\n", timeBuf,
+           log_entry.entry.nsec / 1000000, log_entry.entry.pid, log_entry.entry.tid, prioChar, tag,
+           msg);
     } while ((msg = nl));
   }