commit 3253f2d6f3c2b1b27871aa291b0efd5db97c8c48
author Andy Hung <hunga@google.com> Mon Oct 21 14:50:07 2019 -0700
committer Andy Hung <hunga@google.com> Thu Oct 31 17:43:51 2019 -0700
tree c9c75eda60ccc0ab5ea1bc71704ae0f954fd6fed
parent afa8feb251a71eebce8c659a25202037232fbe59

MediaMetrics: Update Item serialization code

Add for bounds checking errors
Update key checks to omit length, use C primitive types

Test: atest mediametrics_tests, adb shell dumpsys media.metrics
Change-Id: I1e6d5bd7a9611f6d59e15a1dbebc646405e5a018

media/libmediametrics/IMediaAnalyticsService.cpp

diff --git a/media/libmediametrics/IMediaAnalyticsService.cpp b/media/libmediametrics/IMediaAnalyticsService.cpp
index 1ab6653..4324f6d 100644
--- a/media/libmediametrics/IMediaAnalyticsService.cpp
+++ b/media/libmediametrics/IMediaAnalyticsService.cpp
@@ -55,13 +55,17 @@
 
         Parcel data;
         data.writeInterfaceToken(IMediaAnalyticsService::getInterfaceDescriptor());
-        item->writeToParcel(&data);
 
-        status_t err = remote()->transact(
+        status_t status = item->writeToParcel(&data);
+        if (status != NO_ERROR) { // assume failure logged in item
+            return status;
+        }
+
+        status = remote()->transact(
                 SUBMIT_ITEM_ONEWAY, data, nullptr /* reply */, IBinder::FLAG_ONEWAY);
-        ALOGW_IF(err != NO_ERROR, "%s: bad response from service for submit, err=%d",
-                __func__, err);
-        return err;
+        ALOGW_IF(status != NO_ERROR, "%s: bad response from service for submit, status=%d",
+                __func__, status);
+        return status;
     }
 };
 
@@ -79,11 +83,14 @@
         CHECK_INTERFACE(IMediaAnalyticsService, data, reply);
 
         MediaAnalyticsItem * const item = MediaAnalyticsItem::create();
-        if (item->readFromParcel(data) < 0) {
-            return BAD_VALUE;
+        status_t status = item->readFromParcel(data);
+        if (status != NO_ERROR) { // assume failure logged in item
+            return status;
         }
+        // TODO: remove this setPid.
         item->setPid(clientPid);
-        const status_t status __unused = submitInternal(item, true /* release */);
+        status = submitInternal(item, true /* release */);
+        // assume failure logged by submitInternal
         return NO_ERROR;
     } break;