commit bbbf089137ed0ada7589606b392354d77e40f2df
author Tom Cherry <tomcherry@google.com> Wed Oct 09 10:53:37 2019 -0700
committer Tom Cherry <tomcherry@google.com> Wed Oct 09 18:20:49 2019 -0700
tree b85bfb5365ac451fd02444210e54460b4a1e28b0
parent b18dc5d287f1e71cb35d308a0229a4770ade9cf2

liblog: use packed structs instead of raw unaligned reads

Per jmgao@, we still need to worry about unaligned integer accesses.

In any case, we already have the packed structs for all of the
unaligned data that we're reading, so this change favors using those
packed structs.

Bug: 142256213
Test: x86,arm32,arm64 liblog-unit-tests
Change-Id: I21fc629eac49895d03b5b31daa4cc494b0c4c230

liblog/logprint.cpp

diff --git a/liblog/logprint.cpp b/liblog/logprint.cpp
index dd2c797..82fbafd 100644
--- a/liblog/logprint.cpp
+++ b/liblog/logprint.cpp
@@ -35,8 +35,10 @@
 #include <wchar.h>
 
 #include <cutils/list.h>
+
 #include <log/log.h>
 #include <log/logprint.h>
+#include <private/android_logger.h>
 
 #include "log_portability.h"
 
@@ -635,8 +637,7 @@
 
   if (eventDataLen < 1) return -1;
 
-  type = *eventData++;
-  eventDataLen--;
+  type = *eventData;
 
   cp = NULL;
   len = 0;
@@ -725,22 +726,24 @@
     case EVENT_TYPE_INT:
       /* 32-bit signed int */
       {
-        int32_t ival;
-
-        if (eventDataLen < 4) return -1;
-        ival = *reinterpret_cast<const int32_t*>(eventData);
-        eventData += 4;
-        eventDataLen -= 4;
-
-        lval = ival;
+        if (eventDataLen < sizeof(android_event_int_t)) return -1;
+        auto* event_int = reinterpret_cast<const android_event_int_t*>(eventData);
+        lval = event_int->data;
+        eventData += sizeof(android_event_int_t);
+        eventDataLen -= sizeof(android_event_int_t);
       }
       goto pr_lval;
     case EVENT_TYPE_LONG:
       /* 64-bit signed long */
-      if (eventDataLen < 8) return -1;
-      lval = *reinterpret_cast<const int64_t*>(eventData);
-      eventData += 8;
-      eventDataLen -= 8;
+      if (eventDataLen < sizeof(android_event_long_t)) {
+        return -1;
+      }
+      {
+        auto* event_long = reinterpret_cast<const android_event_long_t*>(eventData);
+        lval = event_long->data;
+      }
+      eventData += sizeof(android_event_long_t);
+      eventDataLen -= sizeof(android_event_long_t);
     pr_lval:
       outCount = snprintf(outBuf, outBufLen, "%" PRId64, lval);
       if (outCount < outBufLen) {
@@ -754,14 +757,11 @@
     case EVENT_TYPE_FLOAT:
       /* float */
       {
-        uint32_t ival;
-        float fval;
-
-        if (eventDataLen < 4) return -1;
-        ival = *reinterpret_cast<const uint32_t*>(eventData);
-        fval = *(float*)&ival;
-        eventData += 4;
-        eventDataLen -= 4;
+        if (eventDataLen < sizeof(android_event_float_t)) return -1;
+        auto* event_float = reinterpret_cast<const android_event_float_t*>(eventData);
+        float fval = event_float->data;
+        eventData += sizeof(android_event_int_t);
+        eventDataLen -= sizeof(android_event_int_t);
 
         outCount = snprintf(outBuf, outBufLen, "%f", fval);
         if (outCount < outBufLen) {
@@ -776,12 +776,11 @@
     case EVENT_TYPE_STRING:
       /* UTF-8 chars, not NULL-terminated */
       {
-        unsigned int strLen;
-
-        if (eventDataLen < 4) return -1;
-        strLen = *reinterpret_cast<const uint32_t*>(eventData);
-        eventData += 4;
-        eventDataLen -= 4;
+        if (eventDataLen < sizeof(android_event_string_t)) return -1;
+        auto* event_string = reinterpret_cast<const android_event_string_t*>(eventData);
+        unsigned int strLen = event_string->length;
+        eventData += sizeof(android_event_string_t);
+        eventDataLen -= sizeof(android_event_string_t);
 
         if (eventDataLen < strLen) {
           result = -1; /* mark truncated */
@@ -814,20 +813,19 @@
     case EVENT_TYPE_LIST:
       /* N items, all different types */
       {
-        unsigned char count;
-        int i;
+        if (eventDataLen < sizeof(android_event_list_t)) return -1;
+        auto* event_list = reinterpret_cast<const android_event_list_t*>(eventData);
 
-        if (eventDataLen < 1) return -1;
-
-        count = *eventData++;
-        eventDataLen--;
+        int8_t count = event_list->element_count;
+        eventData += sizeof(android_event_list_t);
+        eventDataLen -= sizeof(android_event_list_t);
 
         if (outBufLen <= 0) goto no_room;
 
         *outBuf++ = '[';
         outBufLen--;
 
-        for (i = 0; i < count; i++) {
+        for (int i = 0; i < count; i++) {
           result = android_log_printBinaryEvent(&eventData, &eventDataLen, &outBuf, &outBufLen,
                                                 fmtStr, fmtLen);
           if (result != 0) goto bail;
@@ -1017,10 +1015,11 @@
     }
   }
   inCount = buf->len;
-  if (inCount < 4) return -1;
-  tagIndex = *reinterpret_cast<const uint32_t*>(eventData);
-  eventData += 4;
-  inCount -= 4;
+  if (inCount < sizeof(android_event_header_t)) return -1;
+  auto* event_header = reinterpret_cast<const android_event_header_t*>(eventData);
+  tagIndex = event_header->tag;
+  eventData += sizeof(android_event_header_t);
+  inCount -= sizeof(android_event_header_t);
 
   entry->tagLen = 0;
   entry->tag = NULL;