Fix getInterfaceVersion() inconsistency bug between backends
Java:
- Server
writeNoException(), and then writeInt(version)
- Client
As-is: just readInt() -> read exception code, not version
To-be: readException, and then readInt() -> consume exception code first
Cpp:
- Transaction code
As-is: getInterface's ID
To-be: FIRST_CALL_TRANSACTION + the ID
- Server & Client
As-is: No exception read/write before version code
To-be: Add exception value before version code
Bug: 133118233
Test: m
Test: atest CtsNdkBinderTestCases
Test: ./runtests.sh
Change-Id: I168a0b8ff7e1871afd95a0ab76b544bfe131a9ed
Merged-In: I168a0b8ff7e1871afd95a0ab76b544bfe131a9ed
diff --git a/aidl_to_cpp.cpp b/aidl_to_cpp.cpp
index 61917e9..f335475 100644
--- a/aidl_to_cpp.cpp
+++ b/aidl_to_cpp.cpp
@@ -38,9 +38,7 @@
std::string GetTransactionIdFor(const AidlMethod& method) {
ostringstream output;
- if (method.IsUserDefined()) {
- output << "::android::IBinder::FIRST_CALL_TRANSACTION + ";
- }
+ output << "::android::IBinder::FIRST_CALL_TRANSACTION + ";
output << method.GetId() << " /* " << method.GetName() << " */";
return output.str();
}
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index 6d08eac..9d2a1fb 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -415,7 +415,11 @@
<< " ::android::status_t err = remote()->transact(" << GetTransactionIdFor(method)
<< ", data, &reply);\n"
<< " if (err == ::android::OK) {\n"
- << " cached_version_ = reply.readInt32();\n"
+ << " ::android::binder::Status _aidl_status;\n"
+ << " err = _aidl_status.readFromParcel(reply);\n"
+ << " if (err == ::android::OK && _aidl_status.isOk()) {\n"
+ << " cached_version_ = reply.readInt32();\n"
+ << " }\n"
<< " }\n"
<< " }\n"
<< " return cached_version_;\n"
@@ -612,6 +616,7 @@
if (method.GetName() == kGetInterfaceVersion && options.Version() > 0) {
std::ostringstream code;
code << "_aidl_data.checkInterface(this);\n"
+ << "_aidl_reply->writeNoException();\n"
<< "_aidl_reply->writeInt32(" << ClassName(interface, ClassNames::INTERFACE)
<< "::VERSION)";
b->AddLiteral(code.str());
diff --git a/generate_java_binder.cpp b/generate_java_binder.cpp
index f017d62..8ced119 100644
--- a/generate_java_binder.cpp
+++ b/generate_java_binder.cpp
@@ -828,6 +828,7 @@
<< " data.writeInterfaceToken(DESCRIPTOR);\n"
<< " mRemote.transact(Stub." << transactCodeName << ", "
<< "data, reply, 0);\n"
+ << " reply.readException();\n"
<< " mCachedVersion = reply.readInt();\n"
<< " } finally {\n"
<< " reply.recycle();\n"
diff --git a/tests/test_data_example_interface.cpp b/tests/test_data_example_interface.cpp
index ef8dbed..24fee69 100644
--- a/tests/test_data_example_interface.cpp
+++ b/tests/test_data_example_interface.cpp
@@ -2518,6 +2518,7 @@
try {
data.writeInterfaceToken(DESCRIPTOR);
mRemote.transact(Stub.TRANSACTION_getInterfaceVersion, data, reply, 0);
+ reply.readException();
mCachedVersion = reply.readInt();
} finally {
reply.recycle();
diff --git a/tests/test_data_ping_responder.cpp b/tests/test_data_ping_responder.cpp
index 6dec57b..9699d26 100644
--- a/tests/test_data_ping_responder.cpp
+++ b/tests/test_data_ping_responder.cpp
@@ -690,9 +690,13 @@
::android::Parcel data;
::android::Parcel reply;
data.writeInterfaceToken(getInterfaceDescriptor());
- ::android::status_t err = remote()->transact(16777214 /* getInterfaceVersion */, data, &reply);
+ ::android::status_t err = remote()->transact(::android::IBinder::FIRST_CALL_TRANSACTION + 16777214 /* getInterfaceVersion */, data, &reply);
if (err == ::android::OK) {
- cached_version_ = reply.readInt32();
+ ::android::binder::Status _aidl_status;
+ err = _aidl_status.readFromParcel(reply);
+ if (err == ::android::OK && _aidl_status.isOk()) {
+ cached_version_ = reply.readInt32();
+ }
}
}
return cached_version_;
@@ -815,9 +819,10 @@
}
}
break;
- case 16777214 /* getInterfaceVersion */:
+ case ::android::IBinder::FIRST_CALL_TRANSACTION + 16777214 /* getInterfaceVersion */:
{
_aidl_data.checkInterface(this);
+ _aidl_reply->writeNoException();
_aidl_reply->writeInt32(IPingResponder::VERSION);
}
break;
diff --git a/tests/test_data_string_constants.cpp b/tests/test_data_string_constants.cpp
index a83874c..2077880 100644
--- a/tests/test_data_string_constants.cpp
+++ b/tests/test_data_string_constants.cpp
@@ -323,6 +323,7 @@
try {
data.writeInterfaceToken(DESCRIPTOR);
mRemote.transact(Stub.TRANSACTION_getInterfaceVersion, data, reply, 0);
+ reply.readException();
mCachedVersion = reply.readInt();
} finally {
reply.recycle();
@@ -429,9 +430,13 @@
::android::Parcel data;
::android::Parcel reply;
data.writeInterfaceToken(getInterfaceDescriptor());
- ::android::status_t err = remote()->transact(16777214 /* getInterfaceVersion */, data, &reply);
+ ::android::status_t err = remote()->transact(::android::IBinder::FIRST_CALL_TRANSACTION + 16777214 /* getInterfaceVersion */, data, &reply);
if (err == ::android::OK) {
- cached_version_ = reply.readInt32();
+ ::android::binder::Status _aidl_status;
+ err = _aidl_status.readFromParcel(reply);
+ if (err == ::android::OK && _aidl_status.isOk()) {
+ cached_version_ = reply.readInt32();
+ }
}
}
return cached_version_;
@@ -450,9 +455,10 @@
::android::status_t BnStringConstants::onTransact(uint32_t _aidl_code, const ::android::Parcel& _aidl_data, ::android::Parcel* _aidl_reply, uint32_t _aidl_flags) {
::android::status_t _aidl_ret_status = ::android::OK;
switch (_aidl_code) {
- case 16777214 /* getInterfaceVersion */:
+ case ::android::IBinder::FIRST_CALL_TRANSACTION + 16777214 /* getInterfaceVersion */:
{
_aidl_data.checkInterface(this);
+ _aidl_reply->writeNoException();
_aidl_reply->writeInt32(IStringConstants::VERSION);
}
break;