Merge "liblog: use packed structs instead of raw unaligned reads"
am: cad2fc2429

Change-Id: I71f8c128207bf6690e94f5845f0cf98771000bd9
diff --git a/liblog/include/private/android_logger.h b/liblog/include/private/android_logger.h
index 5e04148..d3cb571 100644
--- a/liblog/include/private/android_logger.h
+++ b/liblog/include/private/android_logger.h
@@ -57,6 +57,18 @@
   int32_t tag;  // Little Endian Order
 } android_event_header_t;
 
+// Event payload EVENT_TYPE_LIST
+typedef struct __attribute__((__packed__)) {
+  int8_t type;  // EVENT_TYPE_LIST
+  int8_t element_count;
+} android_event_list_t;
+
+// Event payload EVENT_TYPE_FLOAT
+typedef struct __attribute__((__packed__)) {
+  int8_t type;  // EVENT_TYPE_FLOAT
+  float data;
+} android_event_float_t;
+
 /* Event payload EVENT_TYPE_INT */
 typedef struct __attribute__((__packed__)) {
   int8_t type;   // EVENT_TYPE_INT
diff --git a/liblog/log_event_list.cpp b/liblog/log_event_list.cpp
index 18ea930..7882c96 100644
--- a/liblog/log_event_list.cpp
+++ b/liblog/log_event_list.cpp
@@ -51,11 +51,9 @@
 typedef struct android_log_context_internal android_log_context_internal;
 
 static void init_context(android_log_context_internal* context, uint32_t tag) {
-  size_t needed;
-
   context->tag = tag;
   context->read_write_flag = kAndroidLoggerWrite;
-  needed = sizeof(uint8_t) + sizeof(uint8_t);
+  size_t needed = sizeof(android_event_list_t);
   if ((context->pos + needed) > MAX_EVENT_PAYLOAD) {
     context->overflow = true;
   }
@@ -143,7 +141,6 @@
 }
 
 int android_log_write_list_begin(android_log_context ctx) {
-  size_t needed;
   android_log_context_internal* context;
 
   context = (android_log_context_internal*)ctx;
@@ -154,7 +151,7 @@
     context->overflow = true;
     return -EOVERFLOW;
   }
-  needed = sizeof(uint8_t) + sizeof(uint8_t);
+  size_t needed = sizeof(android_event_list_t);
   if ((context->pos + needed) > MAX_EVENT_PAYLOAD) {
     context->overflow = true;
     return -EIO;
@@ -168,8 +165,9 @@
   if (context->overflow) {
     return -EIO;
   }
-  context->storage[context->pos + 0] = EVENT_TYPE_LIST;
-  context->storage[context->pos + 1] = 0;
+  auto* event_list = reinterpret_cast<android_event_list_t*>(&context->storage[context->pos]);
+  event_list->type = EVENT_TYPE_LIST;
+  event_list->element_count = 0;
   context->list[context->list_nest_depth] = context->pos + 1;
   context->count[context->list_nest_depth] = 0;
   context->pos += needed;
@@ -177,57 +175,49 @@
 }
 
 int android_log_write_int32(android_log_context ctx, int32_t value) {
-  size_t needed;
-  android_log_context_internal* context;
-
-  context = (android_log_context_internal*)ctx;
+  android_log_context_internal* context = (android_log_context_internal*)ctx;
   if (!context || (kAndroidLoggerWrite != context->read_write_flag)) {
     return -EBADF;
   }
   if (context->overflow) {
     return -EIO;
   }
-  needed = sizeof(uint8_t) + sizeof(value);
+  size_t needed = sizeof(android_event_int_t);
   if ((context->pos + needed) > MAX_EVENT_PAYLOAD) {
     context->overflow = true;
     return -EIO;
   }
   context->count[context->list_nest_depth]++;
-  context->storage[context->pos + 0] = EVENT_TYPE_INT;
-  *reinterpret_cast<int32_t*>(&context->storage[context->pos + 1]) = value;
+  auto* event_int = reinterpret_cast<android_event_int_t*>(&context->storage[context->pos]);
+  event_int->type = EVENT_TYPE_INT;
+  event_int->data = value;
   context->pos += needed;
   return 0;
 }
 
 int android_log_write_int64(android_log_context ctx, int64_t value) {
-  size_t needed;
-  android_log_context_internal* context;
-
-  context = (android_log_context_internal*)ctx;
+  android_log_context_internal* context = (android_log_context_internal*)ctx;
   if (!context || (kAndroidLoggerWrite != context->read_write_flag)) {
     return -EBADF;
   }
   if (context->overflow) {
     return -EIO;
   }
-  needed = sizeof(uint8_t) + sizeof(value);
+  size_t needed = sizeof(android_event_long_t);
   if ((context->pos + needed) > MAX_EVENT_PAYLOAD) {
     context->overflow = true;
     return -EIO;
   }
   context->count[context->list_nest_depth]++;
-  context->storage[context->pos + 0] = EVENT_TYPE_LONG;
-  *reinterpret_cast<int64_t*>(&context->storage[context->pos + 1]) = value;
+  auto* event_long = reinterpret_cast<android_event_long_t*>(&context->storage[context->pos]);
+  event_long->type = EVENT_TYPE_LONG;
+  event_long->data = value;
   context->pos += needed;
   return 0;
 }
 
 int android_log_write_string8_len(android_log_context ctx, const char* value, size_t maxlen) {
-  size_t needed;
-  ssize_t len;
-  android_log_context_internal* context;
-
-  context = (android_log_context_internal*)ctx;
+  android_log_context_internal* context = (android_log_context_internal*)ctx;
   if (!context || (kAndroidLoggerWrite != context->read_write_flag)) {
     return -EBADF;
   }
@@ -237,8 +227,8 @@
   if (!value) {
     value = "";
   }
-  len = strnlen(value, maxlen);
-  needed = sizeof(uint8_t) + sizeof(int32_t) + len;
+  int32_t len = strnlen(value, maxlen);
+  size_t needed = sizeof(android_event_string_t) + len;
   if ((context->pos + needed) > MAX_EVENT_PAYLOAD) {
     /* Truncate string for delivery */
     len = MAX_EVENT_PAYLOAD - context->pos - 1 - sizeof(int32_t);
@@ -248,10 +238,11 @@
     }
   }
   context->count[context->list_nest_depth]++;
-  context->storage[context->pos + 0] = EVENT_TYPE_STRING;
-  *reinterpret_cast<ssize_t*>(&context->storage[context->pos + 1]) = len;
+  auto* event_string = reinterpret_cast<android_event_string_t*>(&context->storage[context->pos]);
+  event_string->type = EVENT_TYPE_STRING;
+  event_string->length = len;
   if (len) {
-    memcpy(&context->storage[context->pos + 5], value, len);
+    memcpy(&event_string->data, value, len);
   }
   context->pos += needed;
   return len;
@@ -262,26 +253,22 @@
 }
 
 int android_log_write_float32(android_log_context ctx, float value) {
-  size_t needed;
-  uint32_t ivalue;
-  android_log_context_internal* context;
-
-  context = (android_log_context_internal*)ctx;
+  android_log_context_internal* context = (android_log_context_internal*)ctx;
   if (!context || (kAndroidLoggerWrite != context->read_write_flag)) {
     return -EBADF;
   }
   if (context->overflow) {
     return -EIO;
   }
-  needed = sizeof(uint8_t) + sizeof(ivalue);
+  size_t needed = sizeof(android_event_float_t);
   if ((context->pos + needed) > MAX_EVENT_PAYLOAD) {
     context->overflow = true;
     return -EIO;
   }
-  ivalue = *(uint32_t*)&value;
   context->count[context->list_nest_depth]++;
-  context->storage[context->pos + 0] = EVENT_TYPE_FLOAT;
-  *reinterpret_cast<uint32_t*>(&context->storage[context->pos + 1]) = ivalue;
+  auto* event_float = reinterpret_cast<android_event_float_t*>(&context->storage[context->pos]);
+  event_float->type = EVENT_TYPE_FLOAT;
+  event_float->data = value;
   context->pos += needed;
   return 0;
 }
@@ -443,20 +430,22 @@
     return elem;
   }
 
-  elem.type = static_cast<AndroidEventLogType>(context->storage[pos++]);
+  elem.type = static_cast<AndroidEventLogType>(context->storage[pos]);
   switch ((int)elem.type) {
     case EVENT_TYPE_FLOAT:
     /* Rely on union to translate elem.data.int32 into elem.data.float32 */
     /* FALLTHRU */
-    case EVENT_TYPE_INT:
+    case EVENT_TYPE_INT: {
       elem.len = sizeof(int32_t);
-      if ((pos + elem.len) > context->len) {
+      if ((pos + sizeof(android_event_int_t)) > context->len) {
         elem.type = EVENT_TYPE_UNKNOWN;
         return elem;
       }
-      elem.data.int32 = *reinterpret_cast<int32_t*>(&context->storage[pos]);
+
+      auto* event_int = reinterpret_cast<android_event_int_t*>(&context->storage[pos]);
+      pos += sizeof(android_event_int_t);
+      elem.data.int32 = event_int->data;
       /* common tangeable object suffix */
-      pos += elem.len;
       elem.complete = !context->list_nest_depth && !context->count[0];
       if (!peek) {
         if (!context->count[context->list_nest_depth] ||
@@ -466,16 +455,19 @@
         context->pos = pos;
       }
       return elem;
+    }
 
-    case EVENT_TYPE_LONG:
+    case EVENT_TYPE_LONG: {
       elem.len = sizeof(int64_t);
-      if ((pos + elem.len) > context->len) {
+      if ((pos + sizeof(android_event_long_t)) > context->len) {
         elem.type = EVENT_TYPE_UNKNOWN;
         return elem;
       }
-      elem.data.int64 = *reinterpret_cast<int64_t*>(&context->storage[pos]);
+
+      auto* event_long = reinterpret_cast<android_event_long_t*>(&context->storage[pos]);
+      pos += sizeof(android_event_long_t);
+      elem.data.int64 = event_long->data;
       /* common tangeable object suffix */
-      pos += elem.len;
       elem.complete = !context->list_nest_depth && !context->count[0];
       if (!peek) {
         if (!context->count[context->list_nest_depth] ||
@@ -485,15 +477,22 @@
         context->pos = pos;
       }
       return elem;
+    }
 
-    case EVENT_TYPE_STRING:
-      if ((pos + sizeof(int32_t)) > context->len) {
+    case EVENT_TYPE_STRING: {
+      if ((pos + sizeof(android_event_string_t)) > context->len) {
         elem.type = EVENT_TYPE_UNKNOWN;
         elem.complete = true;
         return elem;
       }
-      elem.len = *reinterpret_cast<int32_t*>(&context->storage[pos]);
-      pos += sizeof(int32_t);
+      auto* event_string = reinterpret_cast<android_event_string_t*>(&context->storage[pos]);
+      pos += sizeof(android_event_string_t);
+      // Wire format is int32_t, but elem.len is uint16_t...
+      if (event_string->length >= UINT16_MAX) {
+        elem.type = EVENT_TYPE_UNKNOWN;
+        return elem;
+      }
+      elem.len = event_string->length;
       if ((pos + elem.len) > context->len) {
         elem.len = context->len - pos; /* truncate string */
         elem.complete = true;
@@ -502,7 +501,7 @@
           return elem;
         }
       }
-      elem.data.string = (char*)&context->storage[pos];
+      elem.data.string = event_string->data;
       /* common tangeable object suffix */
       pos += elem.len;
       elem.complete = !context->list_nest_depth && !context->count[0];
@@ -514,13 +513,16 @@
         context->pos = pos;
       }
       return elem;
+    }
 
-    case EVENT_TYPE_LIST:
-      if ((pos + sizeof(uint8_t)) > context->len) {
+    case EVENT_TYPE_LIST: {
+      if ((pos + sizeof(android_event_list_t)) > context->len) {
         elem.type = EVENT_TYPE_UNKNOWN;
         elem.complete = true;
         return elem;
       }
+      auto* event_list = reinterpret_cast<android_event_list_t*>(&context->storage[pos]);
+      pos += sizeof(android_event_list_t);
       elem.complete = context->list_nest_depth >= ANDROID_MAX_LIST_NEST_DEPTH;
       if (peek) {
         return elem;
@@ -528,15 +530,17 @@
       if (context->count[context->list_nest_depth]) {
         context->count[context->list_nest_depth]--;
       }
-      context->list_stop = !context->storage[pos];
+      context->list_stop = event_list->element_count == 0;
       context->list_nest_depth++;
       if (context->list_nest_depth <= ANDROID_MAX_LIST_NEST_DEPTH) {
-        context->count[context->list_nest_depth] = context->storage[pos];
+        context->count[context->list_nest_depth] = event_list->element_count;
       }
-      context->pos = pos + sizeof(uint8_t);
+      context->pos = pos;
       return elem;
+    }
 
     case EVENT_TYPE_LIST_STOP: /* Suprise Newline terminates lists. */
+      pos++;
       if (!peek) {
         context->pos = pos;
       }
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;
diff --git a/liblog/tests/liblog_test.cpp b/liblog/tests/liblog_test.cpp
index 9780b28..cfcfd99 100644
--- a/liblog/tests/liblog_test.cpp
+++ b/liblog/tests/liblog_test.cpp
@@ -1924,10 +1924,10 @@
     char* original = eventData;
 
     // Tag
-    int tag = *reinterpret_cast<int32_t*>(eventData);
-    eventData += 4;
+    auto* event_header = reinterpret_cast<android_event_header_t*>(eventData);
+    eventData += sizeof(android_event_header_t);
 
-    if (tag != TAG) {
+    if (event_header->tag != TAG) {
       continue;
     }
 
@@ -1938,45 +1938,37 @@
     }
 
     // List type
-    ASSERT_EQ(EVENT_TYPE_LIST, eventData[0]);
-    eventData++;
-
-    // Number of elements in list
-    ASSERT_EQ(3, eventData[0]);
-    eventData++;
+    auto* event_list = reinterpret_cast<android_event_list_t*>(eventData);
+    ASSERT_EQ(EVENT_TYPE_LIST, event_list->type);
+    ASSERT_EQ(3, event_list->element_count);
+    eventData += sizeof(android_event_list_t);
 
     // Element #1: string type for subtag
-    ASSERT_EQ(EVENT_TYPE_STRING, eventData[0]);
-    eventData++;
-
+    auto* event_string_subtag = reinterpret_cast<android_event_string_t*>(eventData);
+    ASSERT_EQ(EVENT_TYPE_STRING, event_string_subtag->type);
     unsigned subtag_len = strlen(SUBTAG);
     if (subtag_len > 32) subtag_len = 32;
-    ASSERT_EQ(subtag_len, *reinterpret_cast<uint32_t*>(eventData));
-    eventData += 4;
-
-    if (memcmp(SUBTAG, eventData, subtag_len)) {
+    ASSERT_EQ(static_cast<int32_t>(subtag_len), event_string_subtag->length);
+    if (memcmp(SUBTAG, &event_string_subtag->data, subtag_len)) {
       continue;
     }
-    eventData += subtag_len;
+    eventData += sizeof(android_event_string_t) + subtag_len;
 
     // Element #2: int type for uid
-    ASSERT_EQ(EVENT_TYPE_INT, eventData[0]);
-    eventData++;
-
-    ASSERT_EQ(UID, *reinterpret_cast<int32_t*>(eventData));
-    eventData += 4;
+    auto* event_int_uid = reinterpret_cast<android_event_int_t*>(eventData);
+    ASSERT_EQ(EVENT_TYPE_INT, event_int_uid->type);
+    ASSERT_EQ(UID, event_int_uid->data);
+    eventData += sizeof(android_event_int_t);
 
     // Element #3: string type for data
-    ASSERT_EQ(EVENT_TYPE_STRING, eventData[0]);
-    eventData++;
-
-    size_t dataLen = *reinterpret_cast<int32_t*>(eventData);
-    eventData += 4;
+    auto* event_string_data = reinterpret_cast<android_event_string_t*>(eventData);
+    ASSERT_EQ(EVENT_TYPE_STRING, event_string_data->type);
+    size_t dataLen = event_string_data->length;
     if (DATA_LEN < 512) ASSERT_EQ(DATA_LEN, (int)dataLen);
-
-    if (memcmp(payload, eventData, dataLen)) {
+    if (memcmp(payload, &event_string_data->data, dataLen)) {
       continue;
     }
+    eventData += sizeof(android_event_string_t);
 
     if (DATA_LEN >= 512) {
       eventData += dataLen;
@@ -2082,10 +2074,12 @@
     if (!eventData) continue;
 
     // Tag
-    int tag = *reinterpret_cast<int32_t*>(eventData);
-    eventData += 4;
+    auto* event_header = reinterpret_cast<android_event_header_t*>(eventData);
+    eventData += sizeof(android_event_header_t);
 
-    if (tag != TAG) continue;
+    if (event_header->tag != TAG) {
+      continue;
+    }
 
     if (!SUBTAG) {
       // This tag should not have been written because the data was null
@@ -2135,10 +2129,10 @@
     }
 
     // Tag
-    int tag = *reinterpret_cast<int32_t*>(eventData);
-    eventData += 4;
+    auto* event_header = reinterpret_cast<android_event_header_t*>(eventData);
+    eventData += sizeof(android_event_header_t);
 
-    if (tag != TAG) {
+    if (event_header->tag != TAG) {
       continue;
     }
 
@@ -2149,21 +2143,17 @@
     }
 
     // List type
-    ASSERT_EQ(EVENT_TYPE_LIST, eventData[0]);
-    eventData++;
-
-    // Number of elements in list
-    ASSERT_EQ(3, eventData[0]);
-    eventData++;
+    auto* event_list = reinterpret_cast<android_event_list_t*>(eventData);
+    ASSERT_EQ(EVENT_TYPE_LIST, event_list->type);
+    ASSERT_EQ(3, event_list->element_count);
+    eventData += sizeof(android_event_list_t);
 
     // Element #1: string type for subtag
-    ASSERT_EQ(EVENT_TYPE_STRING, eventData[0]);
-    eventData++;
+    auto* event_string = reinterpret_cast<android_event_string_t*>(eventData);
+    ASSERT_EQ(EVENT_TYPE_STRING, event_string->type);
+    ASSERT_EQ(static_cast<int32_t>(strlen(SUBTAG)), event_string->length);
 
-    ASSERT_EQ(strlen(SUBTAG), *reinterpret_cast<uint32_t*>(eventData));
-    eventData += 4;
-
-    if (memcmp(SUBTAG, eventData, strlen(SUBTAG))) {
+    if (memcmp(SUBTAG, &event_string->data, strlen(SUBTAG))) {
       continue;
     }
     ++count;
@@ -2673,13 +2663,14 @@
     // test buffer reading API
     int buffer_to_string = -1;
     if (eventData) {
-      snprintf(msgBuf, sizeof(msgBuf), "I/[%" PRIu32 "]", *reinterpret_cast<uint32_t*>(eventData));
+      auto* event_header = reinterpret_cast<android_event_header_t*>(eventData);
+      eventData += sizeof(android_event_header_t);
+      snprintf(msgBuf, sizeof(msgBuf), "I/[%" PRId32 "]", event_header->tag);
       print_barrier();
       fprintf(stderr, "%-10s(%5u): ", msgBuf, pid);
       memset(msgBuf, 0, sizeof(msgBuf));
-      buffer_to_string = android_log_buffer_to_string(
-          eventData + sizeof(uint32_t), log_msg.entry.len - sizeof(uint32_t),
-          msgBuf, sizeof(msgBuf));
+      buffer_to_string =
+          android_log_buffer_to_string(eventData, log_msg.entry.len, msgBuf, sizeof(msgBuf));
       fprintf(stderr, "%s\n", msgBuf);
       print_barrier();
     }