Merge afa7b730dd895418d0c52d7f9bd9b0883ea6df68 on remote branch
Change-Id: I8ab2e5c981e3e6844f491d8dd68ec731c3cfe627
diff --git a/aidl_checkapi.cpp b/aidl_checkapi.cpp
index b22c294..f0228d9 100644
--- a/aidl_checkapi.cpp
+++ b/aidl_checkapi.cpp
@@ -182,15 +182,6 @@
const auto& new_field = new_fields.at(i);
compatible &= are_compatible_types(old_field->GetType(), new_field->GetType());
- // Note: unlike method argument names, field name change is an incompatible
- // change, otherwise, we can't detect
- // parcelable Point {int x; int y;} -> parcelable Point {int y; int x;}
- if (old_field->GetName() != new_field->GetName()) {
- AIDL_ERROR(newer) << "Renamed field: " << old_field->GetName() << " to "
- << new_field->GetName() << ".";
- compatible = false;
- }
-
const string old_value = old_field->ValueString(AidlConstantValueDecorator);
const string new_value = new_field->ValueString(AidlConstantValueDecorator);
if (old_value != new_value) {
@@ -198,6 +189,23 @@
compatible = false;
}
}
+
+ // Reordering of fields is an incompatible change.
+ for (size_t i = 0; i < new_fields.size(); i++) {
+ const auto& new_field = new_fields.at(i);
+ auto found = std::find_if(old_fields.begin(), old_fields.end(), [&new_field](const auto& f) {
+ return new_field->GetName() == f->GetName();
+ });
+ if (found != old_fields.end()) {
+ size_t old_index = std::distance(old_fields.begin(), found);
+ if (old_index != i) {
+ AIDL_ERROR(new_field) << "Reordered " << new_field->GetName() << " from " << old_index
+ << " to " << i << ".";
+ compatible = false;
+ }
+ }
+ }
+
return compatible;
}
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 2cc280e..6827980 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -1386,22 +1386,6 @@
EXPECT_FALSE(::android::aidl::check_api(options_, io_delegate_));
}
-TEST_F(AidlTestIncompatibleChanges, RenamedField) {
- io_delegate_.SetFileContents("old/p/Data.aidl",
- "package p;"
- "parcelable Data {"
- " int foo;"
- " int bar;"
- "}");
- io_delegate_.SetFileContents("new/p/Data.aidl",
- "package p;"
- "parcelable Data {"
- " int foo;"
- " int bar2;"
- "}");
- EXPECT_FALSE(::android::aidl::check_api(options_, io_delegate_));
-}
-
TEST_F(AidlTestIncompatibleChanges, RenamedType) {
io_delegate_.SetFileContents("old/p/IFoo.aidl",
"package p;"
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index 2562a74..cc25657 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -462,7 +462,7 @@
<< " ::android::binder::Status _aidl_status;\n"
<< " err = _aidl_status.readFromParcel(reply);\n"
<< " if (err == ::android::OK && _aidl_status.isOk()) {\n"
- << " cached_hash_ = reply.readString8().c_str();\n"
+ << " reply.readUtf8FromUtf16(&cached_hash_);\n"
<< " }\n"
<< " }\n"
<< " }\n"
@@ -663,8 +663,8 @@
std::ostringstream code;
code << "_aidl_data.checkInterface(this);\n"
<< "_aidl_reply->writeNoException();\n"
- << "_aidl_reply->writeString8(android::String8("
- << ClassName(interface, ClassNames::INTERFACE) << "::HASH.c_str()))";
+ << "_aidl_reply->writeUtf8AsUtf16(" << ClassName(interface, ClassNames::INTERFACE)
+ << "::HASH)";
b->AddLiteral(code.str());
return true;
}
diff --git a/generate_java_binder.cpp b/generate_java_binder.cpp
index 3cf4acd..01e88db 100644
--- a/generate_java_binder.cpp
+++ b/generate_java_binder.cpp
@@ -882,7 +882,7 @@
<< "public synchronized String " << kGetInterfaceHash << "()"
<< " throws "
<< "android.os.RemoteException {\n"
- << " if (mCachedHash == \"-1\") {\n"
+ << " if (\"-1\".equals(mCachedHash)) {\n"
<< " android.os.Parcel data = android.os.Parcel.obtain();\n"
<< " android.os.Parcel reply = android.os.Parcel.obtain();\n"
<< " try {\n"
@@ -1131,7 +1131,13 @@
const string i_name = iface->GetCanonicalName();
stub->elements.emplace_back(std::make_shared<LiteralClassElement>(
StringPrintf("public static boolean setDefaultImpl(%s impl) {\n"
- " if (Stub.Proxy.sDefaultImpl == null && impl != null) {\n"
+ " // Only one user of this interface can use this function\n"
+ " // at a time. This is a heuristic to detect if two different\n"
+ " // users in the same process use this function.\n"
+ " if (Stub.Proxy.sDefaultImpl != null) {\n"
+ " throw new IllegalStateException(\"setDefaultImpl() called twice\");\n"
+ " }\n"
+ " if (impl != null) {\n"
" Stub.Proxy.sDefaultImpl = impl;\n"
" return true;\n"
" }\n"
diff --git a/generate_ndk.cpp b/generate_ndk.cpp
index d08ac1b..274d283 100644
--- a/generate_ndk.cpp
+++ b/generate_ndk.cpp
@@ -623,7 +623,11 @@
// defintion for static member setDefaultImpl
out << "bool " << clazz << "::setDefaultImpl(std::shared_ptr<" << clazz << "> impl) {\n";
out.Indent();
- out << "if (!" << clazz << "::default_impl && impl) {\n";
+ out << "// Only one user of this interface can use this function\n";
+ out << "// at a time. This is a heuristic to detect if two different\n";
+ out << "// users in the same process use this function.\n";
+ out << "assert(!" << clazz << "::default_impl);\n";
+ out << "if (impl) {\n";
out.Indent();
out << clazz << "::default_impl = impl;\n";
out << "return true;\n";
diff --git a/tests/test_data_example_interface.cpp b/tests/test_data_example_interface.cpp
index 6265a87..c9e32bf 100644
--- a/tests/test_data_example_interface.cpp
+++ b/tests/test_data_example_interface.cpp
@@ -549,7 +549,13 @@
static final int TRANSACTION_takesAnInterface = (android.os.IBinder.FIRST_CALL_TRANSACTION + 7);
static final int TRANSACTION_takesAParcelable = (android.os.IBinder.FIRST_CALL_TRANSACTION + 8);
public static boolean setDefaultImpl(android.test.IExampleInterface impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
@@ -1063,7 +1069,13 @@
static final int TRANSACTION_takesAnInterface = (android.os.IBinder.FIRST_CALL_TRANSACTION + 7);
static final int TRANSACTION_takesAParcelable = (android.os.IBinder.FIRST_CALL_TRANSACTION + 8);
public static boolean setDefaultImpl(android.test.IExampleInterface impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
@@ -1605,7 +1617,13 @@
static final int TRANSACTION_takesAnInterface = (android.os.IBinder.FIRST_CALL_TRANSACTION + 7);
static final int TRANSACTION_takesAParcelable = (android.os.IBinder.FIRST_CALL_TRANSACTION + 8);
public static boolean setDefaultImpl(android.test.IExampleInterface impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
@@ -2095,7 +2113,13 @@
return true;
}
public static boolean setDefaultImpl(android.test.IExampleInterface impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
@@ -2547,7 +2571,7 @@
}
@Override
public synchronized String getInterfaceHash() throws android.os.RemoteException {
- if (mCachedHash == "-1") {
+ if ("-1".equals(mCachedHash)) {
android.os.Parcel data = android.os.Parcel.obtain();
android.os.Parcel reply = android.os.Parcel.obtain();
try {
@@ -2661,7 +2685,13 @@
static final int TRANSACTION_getInterfaceVersion = (android.os.IBinder.FIRST_CALL_TRANSACTION + 16777214);
static final int TRANSACTION_getInterfaceHash = (android.os.IBinder.FIRST_CALL_TRANSACTION + 16777213);
public static boolean setDefaultImpl(android.test.IExampleInterface impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
diff --git a/tests/test_data_ping_responder.cpp b/tests/test_data_ping_responder.cpp
index fdc2bd4..e7088fe 100644
--- a/tests/test_data_ping_responder.cpp
+++ b/tests/test_data_ping_responder.cpp
@@ -685,7 +685,7 @@
::android::binder::Status _aidl_status;
err = _aidl_status.readFromParcel(reply);
if (err == ::android::OK && _aidl_status.isOk()) {
- cached_hash_ = reply.readString8().c_str();
+ reply.readUtf8FromUtf16(&cached_hash_);
}
}
}
@@ -826,7 +826,7 @@
{
_aidl_data.checkInterface(this);
_aidl_reply->writeNoException();
- _aidl_reply->writeString8(android::String8(IPingResponder::HASH.c_str()));
+ _aidl_reply->writeUtf8AsUtf16(IPingResponder::HASH);
}
break;
default:
diff --git a/tests/test_data_string_constants.cpp b/tests/test_data_string_constants.cpp
index 0bea929..6ea8a6c 100644
--- a/tests/test_data_string_constants.cpp
+++ b/tests/test_data_string_constants.cpp
@@ -108,7 +108,13 @@
public static android.os.IStringConstants sDefaultImpl;
}
public static boolean setDefaultImpl(android.os.IStringConstants impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
@@ -355,7 +361,7 @@
}
@Override
public synchronized String getInterfaceHash() throws android.os.RemoteException {
- if (mCachedHash == "-1") {
+ if ("-1".equals(mCachedHash)) {
android.os.Parcel data = android.os.Parcel.obtain();
android.os.Parcel reply = android.os.Parcel.obtain();
try {
@@ -380,7 +386,13 @@
static final int TRANSACTION_getInterfaceVersion = (android.os.IBinder.FIRST_CALL_TRANSACTION + 16777214);
static final int TRANSACTION_getInterfaceHash = (android.os.IBinder.FIRST_CALL_TRANSACTION + 16777213);
public static boolean setDefaultImpl(android.os.IStringConstants impl) {
- if (Stub.Proxy.sDefaultImpl == null && impl != null) {
+ // Only one user of this interface can use this function
+ // at a time. This is a heuristic to detect if two different
+ // users in the same process use this function.
+ if (Stub.Proxy.sDefaultImpl != null) {
+ throw new IllegalStateException("setDefaultImpl() called twice");
+ }
+ if (impl != null) {
Stub.Proxy.sDefaultImpl = impl;
return true;
}
@@ -499,7 +511,7 @@
::android::binder::Status _aidl_status;
err = _aidl_status.readFromParcel(reply);
if (err == ::android::OK && _aidl_status.isOk()) {
- cached_hash_ = reply.readString8().c_str();
+ reply.readUtf8FromUtf16(&cached_hash_);
}
}
}
@@ -536,7 +548,7 @@
{
_aidl_data.checkInterface(this);
_aidl_reply->writeNoException();
- _aidl_reply->writeString8(android::String8(IStringConstants::HASH.c_str()));
+ _aidl_reply->writeUtf8AsUtf16(IStringConstants::HASH);
}
break;
default: