From d2bcf2fea80501be334d15e54d7900b997881293 Mon Sep 17 00:00:00 2001 From: Casey Dahlin Date: Wed, 16 Aug 2017 14:24:16 -0700 Subject: Support nullable String arrays Declaring a String[] used to cause compiler errors due to occasionally treating String as a primitive type and thus not handling the fact that the elements are also nullable. We've now fixed that. Test: New unit tests pass Bug: 64681043 Change-Id: I549610b5c800b3c9e5aca8c366dbbedfd3a9a329 --- tests/aidl_test_client_utf8_strings.cpp | 21 ++++++++++++-- tests/aidl_test_service.cpp | 8 ++++++ tests/android/aidl/tests/ITestService.aidl | 4 +++ .../src/android/aidl/tests/TestServiceClient.java | 32 ++++++++++++++++++++++ type_cpp.cpp | 17 +++++++----- 5 files changed, 73 insertions(+), 9 deletions(-) diff --git a/tests/aidl_test_client_utf8_strings.cpp b/tests/aidl_test_client_utf8_strings.cpp index 216aef7..3688a32 100644 --- a/tests/aidl_test_client_utf8_strings.cpp +++ b/tests/aidl_test_client_utf8_strings.cpp @@ -110,10 +110,16 @@ bool ConfirmUtf8InCppStringArrayReverse(const sp& s) { return true; } -bool ConfirmUtf8InCppStringListReverse(const sp& s) { +namespace { + +bool ConfirmUtf8InCppStringCollectionReverse(const sp& s, + Status (ITestService::*m)(const unique_ptr>>&, + unique_ptr>>*, + unique_ptr>>*)) { + (void)m; LOG(INFO) << "Confirming reversing a list of utf8 strings works"; unique_ptr>> input, reversed, repeated; - Status status = s->ReverseUtf8CppStringList(input, &reversed, &repeated); + Status status = (s.get()->*m)(input, &reversed, &repeated); if (!status.isOk() || reversed || repeated) { LOG(ERROR) << "Reversing null list of utf8 strings failed."; return false; @@ -160,6 +166,17 @@ bool ConfirmUtf8InCppStringListReverse(const sp& s) { return true; } +} // namespace + +bool ConfirmUtf8InCppStringListReverse(const sp& s) { + return ConfirmUtf8InCppStringCollectionReverse(s, + &ITestService::ReverseUtf8CppStringList); +} + +bool ConfirmUtf8InCppNullableStringArrayReverse(const sp& s) { + return ConfirmUtf8InCppStringCollectionReverse(s, + &ITestService::ReverseNullableUtf8CppString); +} } // namespace client } // namespace tests diff --git a/tests/aidl_test_service.cpp b/tests/aidl_test_service.cpp index abea6ae..c5e4bcd 100644 --- a/tests/aidl_test_service.cpp +++ b/tests/aidl_test_service.cpp @@ -399,6 +399,14 @@ class NativeService : public BnTestService { return ReverseArray(input, repeated, _aidl_return); } + Status ReverseNullableUtf8CppString( + const unique_ptr>>& input, + unique_ptr>>* repeated, + unique_ptr>>* _aidl_return) { + + return ReverseUtf8CppStringList(input, repeated, _aidl_return); + } + Status ReverseUtf8CppStringList( const unique_ptr>>& input, unique_ptr>>* repeated, diff --git a/tests/android/aidl/tests/ITestService.aidl b/tests/android/aidl/tests/ITestService.aidl index 91b13b4..1f2ae8c 100644 --- a/tests/android/aidl/tests/ITestService.aidl +++ b/tests/android/aidl/tests/ITestService.aidl @@ -104,6 +104,10 @@ interface ITestService { @utf8InCpp String[] ReverseUtf8CppString (in @utf8InCpp String[] input, out @utf8InCpp String[] repeated); + @nullable @utf8InCpp String[] ReverseNullableUtf8CppString ( + in @nullable @utf8InCpp String[] input, + out @nullable @utf8InCpp String[] repeated); + @nullable @utf8InCpp List ReverseUtf8CppStringList( in @nullable @utf8InCpp List input, out @nullable @utf8InCpp List repeated); diff --git a/tests/java_app/src/android/aidl/tests/TestServiceClient.java b/tests/java_app/src/android/aidl/tests/TestServiceClient.java index 4f4d462..04ca2e9 100644 --- a/tests/java_app/src/android/aidl/tests/TestServiceClient.java +++ b/tests/java_app/src/android/aidl/tests/TestServiceClient.java @@ -631,6 +631,14 @@ public class TestServiceClient extends Activity { "\0\0\0", // Java doesn't handle unicode code points above U+FFFF well. new String(Character.toChars(0x1F701)) + "\u03A9"); + final List utf8_queries_and_nulls = Arrays.asList( + "typical string", + null, + "", + "\0\0\0", + null, + // Java doesn't handle unicode code points above U+FFFF well. + new String(Character.toChars(0x1F701)) + "\u03A9"); try { for (String query : utf8_queries) { String response = service.RepeatUtf8CppString(query); @@ -660,6 +668,30 @@ public class TestServiceClient extends Activity { } } } + { + String[] input = (String[])utf8_queries_and_nulls.toArray(); + String echoed[] = new String[input.length]; + String[] reversed = service.ReverseNullableUtf8CppString(input, + echoed); + if (!Arrays.equals(input, echoed)) { + mLog.logAndThrow("Failed to echo utf8 input array back."); + } + if (input.length != reversed.length) { + mLog.logAndThrow("Reversed utf8 array is the wrong size."); + } + for (int i = 0; i < input.length; ++i) { + int j = reversed.length - (1 + i); + if (input[i] == null && reversed[j] == null) { + continue; + } + + if (!input[i].equals(reversed[j])) { + mLog.logAndThrow( + "input[" + i + "] = " + input[i] + + " but reversed value = " + reversed[j]); + } + } + } } catch (RemoteException ex) { mLog.log(ex.toString()); mLog.logAndThrow("Service failed to handle utf8 strings."); diff --git a/type_cpp.cpp b/type_cpp.cpp index 2ac1523..6159231 100644 --- a/type_cpp.cpp +++ b/type_cpp.cpp @@ -83,6 +83,7 @@ class CppArrayType : public Type { const string& underlying_aidl_type, const string& cpp_header, const string& underlying_cpp_type, + const string& underlying_cpp_type_nulllable, const string& read_method, const string& write_method, bool is_nullable, @@ -96,7 +97,8 @@ class CppArrayType : public Type { ? kNoNullableType // All arrays are nullable. : new CppArrayType(kind, package, underlying_aidl_type, - cpp_header, underlying_cpp_type, + cpp_header, underlying_cpp_type_nulllable, + underlying_cpp_type_nulllable, read_method, write_method, true), src_file_name) {} @@ -138,7 +140,7 @@ class PrimitiveType : public Type { : Type(ValidatableType::KIND_BUILT_IN, kNoPackage, aidl_type, {header}, cpp_type, read_method, write_method, new CppArrayType(ValidatableType::KIND_BUILT_IN, kNoPackage, - aidl_type, header, cpp_type, + aidl_type, header, cpp_type, cpp_type, read_array_method, write_array_method, false)) {} @@ -158,7 +160,7 @@ class ByteType : public Type { : Type(ValidatableType::KIND_BUILT_IN, kNoPackage, "byte", {"cstdint"}, "int8_t", "readByte", "writeByte", new CppArrayType(ValidatableType::KIND_BUILT_IN, kNoPackage, - "byte", "cstdint", "uint8_t", + "byte", "cstdint", "uint8_t", "uint8_t", "readByteVector", "writeByteVector", false)) {} @@ -250,7 +252,7 @@ class ParcelableType : public Type { new CppArrayType( ValidatableType::KIND_PARCELABLE, parcelable.GetPackage(), parcelable.GetName(), parcelable.GetCppHeader(), - GetCppName(parcelable), + GetCppName(parcelable), GetCppName(parcelable), "readParcelableVector", "writeParcelableVector", false, src_file_name), new NullableParcelableType(parcelable, src_file_name), @@ -441,7 +443,8 @@ void TypeNamespace::Init() { Type* string_array_type = new CppArrayType( ValidatableType::KIND_BUILT_IN, "java.lang", "String", "utils/String16.h", "::android::String16", - "readString16Vector", "writeString16Vector", false); + "::std::unique_ptr<::android::String16>", "readString16Vector", + "writeString16Vector", false); Type* nullable_string_type = new Type(ValidatableType::KIND_BUILT_IN, "java.lang", "String", @@ -462,7 +465,7 @@ void TypeNamespace::Init() { Type* cpp_utf8_string_array = new CppArrayType( ValidatableType::KIND_BUILT_IN, kAidlReservedTypePackage, kUtf8InCppStringClass, - "string", "::std::string", + "string", "::std::string", "::std::unique_ptr<::std::string>", "readUtf8VectorFromUtf16Vector", "writeUtf8VectorAsUtf16Vector", false); Type* nullable_cpp_utf8_string_type = new Type( @@ -496,7 +499,7 @@ void TypeNamespace::Init() { Type* fd_vector_type = new CppArrayType( ValidatableType::KIND_BUILT_IN, kNoPackage, "FileDescriptor", "android-base/unique_fd.h", - "::android::base::unique_fd", + "::android::base::unique_fd", "::android::base::unique_fd", "readUniqueFileDescriptorVector", "writeUniqueFileDescriptorVector", false); -- cgit v1.2.3