commit b4e1461cb31bbd40a1cddf4c80bb020b36c6493f
author Steven Moreland <smoreland@google.com> Wed Nov 14 17:25:45 2018 -0800
committer Steven Moreland <smoreland@google.com> Wed Nov 14 18:00:16 2018 -0800
tree 805733a44cb8e657f65de287d0ae2ae7f2217b8d
parent 0b8a3c363da75c86bfbe631b8632bd2ff67238f3

libbinder_ndk: reading strings is optional.

This is converting the most complicated type in the NDK layer to be
optional. This is for feature parity with Java. In AIDL itself, like in
the C++ backend, @nullable will be used to determine whether the C++
layer above the NDK actually accepts the nullable types.

Bug: 112664205
Test: atest android.binder.cts
Change-Id: Ia020706318956a008de5e715730725e9a5a1308d

libs/binder/ndk/parcel.cpp

diff --git a/libs/binder/ndk/parcel.cpp b/libs/binder/ndk/parcel.cpp
index 53e5010..4136698 100644
--- a/libs/binder/ndk/parcel.cpp
+++ b/libs/binder/ndk/parcel.cpp
@@ -237,9 +237,23 @@
     return PruneStatusT(ret);
 }
 
-binder_status_t AParcel_writeString(AParcel* parcel, const char* string, size_t length) {
-    const uint8_t* str8 = (uint8_t*)string;
+binder_status_t AParcel_writeString(AParcel* parcel, const char* string, int32_t length) {
+    if (string == nullptr) {
+        if (length != -1) {
+            LOG(WARNING) << __func__ << ": null string must be used with length == -1.";
+            return STATUS_BAD_VALUE;
+        }
 
+        status_t err = parcel->get()->writeInt32(-1);
+        return PruneStatusT(err);
+    }
+
+    if (length < 0) {
+        LOG(WARNING) << __func__ << ": Negative string length: " << length;
+        return STATUS_BAD_VALUE;
+    }
+
+    const uint8_t* str8 = (uint8_t*)string;
     const ssize_t len16 = utf8_to_utf16_length(str8, length);
 
     if (len16 < 0 || len16 >= std::numeric_limits<int32_t>::max()) {
@@ -268,7 +282,10 @@
     const char16_t* str16 = parcel->get()->readString16Inplace(&len16);
 
     if (str16 == nullptr) {
-        LOG(WARNING) << __func__ << ": Failed to read string in place.";
+        if (allocator(stringData, -1, nullptr)) {
+            return STATUS_OK;
+        }
+
         return STATUS_UNEXPECTED_NULL;
     }
 
@@ -285,9 +302,10 @@
         return STATUS_BAD_VALUE;
     }
 
-    char* str8 = allocator(stringData, len8);
+    char* str8;
+    bool success = allocator(stringData, len8, &str8);
 
-    if (str8 == nullptr) {
+    if (!success || str8 == nullptr) {
         LOG(WARNING) << __func__ << ": AParcel_stringAllocator failed to allocate.";
         return STATUS_NO_MEMORY;
     }
@@ -297,19 +315,27 @@
     return STATUS_OK;
 }
 
-binder_status_t AParcel_writeStringArray(AParcel* parcel, const void* arrayData, size_t length,
+binder_status_t AParcel_writeStringArray(AParcel* parcel, const void* arrayData, int32_t length,
                                          AParcel_stringArrayElementGetter getter) {
-    if (length > std::numeric_limits<int32_t>::max()) return STATUS_BAD_VALUE;
-
     Parcel* rawParcel = parcel->get();
 
+    if (length < 0) {
+        if (length != -1) {
+            LOG(WARNING) << __func__ << ": null array must be used with length == -1.";
+            return STATUS_BAD_VALUE;
+        }
+
+        status_t status = rawParcel->writeInt32(-1);
+        return PruneStatusT(status);
+    }
+
     status_t status = rawParcel->writeInt32(static_cast<int32_t>(length));
     if (status != STATUS_OK) return PruneStatusT(status);
 
     for (size_t i = 0; i < length; i++) {
         size_t length = 0;
         const char* str = getter(arrayData, i, &length);
-        if (str == nullptr) return STATUS_BAD_VALUE;
+        if (str == nullptr && length != -1) return STATUS_BAD_VALUE;
 
         binder_status_t status = AParcel_writeString(parcel, str, length);
         if (status != STATUS_OK) return status;
@@ -325,10 +351,10 @@
     size_t index;     // index into the string array
     AParcel_stringArrayElementAllocator elementAllocator;
 
-    static char* Allocator(void* stringData, size_t length) {
+    static bool Allocator(void* stringData, int32_t length, char** buffer) {
         StringArrayElementAllocationAdapter* adapter =
                 static_cast<StringArrayElementAllocationAdapter*>(stringData);
-        return adapter->elementAllocator(adapter->arrayData, adapter->index, length);
+        return adapter->elementAllocator(adapter->arrayData, adapter->index, length, buffer);
     }
 };
 
@@ -341,10 +367,12 @@
     status_t status = rawParcel->readInt32(&length);
 
     if (status != STATUS_OK) return PruneStatusT(status);
-    if (length < 0) return STATUS_UNEXPECTED_NULL;
+    if (length < -1) return STATUS_BAD_VALUE;
 
     if (!allocator(arrayData, length)) return STATUS_NO_MEMORY;
 
+    if (length == -1) return STATUS_OK;  // null string array
+
     StringArrayElementAllocationAdapter adapter{
             .arrayData = arrayData,
             .index = 0,
@@ -352,8 +380,10 @@
     };
 
     for (; adapter.index < length; adapter.index++) {
-        AParcel_readString(parcel, static_cast<void*>(&adapter),
-                           StringArrayElementAllocationAdapter::Allocator);
+        binder_status_t status = AParcel_readString(parcel, static_cast<void*>(&adapter),
+                                                    StringArrayElementAllocationAdapter::Allocator);
+
+        if (status != STATUS_OK) return status;
     }
 
     return STATUS_OK;