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

6 files changed, 26 insertions, 10 deletions

diff --git a/aidl_to_cpp.cpp b/aidl_to_cpp.cpp
index 61917e99..f335475e 100644
--- a/aidl_to_cpp.cpp
+++ b/aidl_to_cpp.cpp
@@ -38,9 +38,7 @@ std::string ConstantValueDecorator(const AidlTypeSpecifier& type, const std::str
 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 6d08eac7..9d2a1fb6 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -415,7 +415,11 @@ unique_ptr<Declaration> DefineClientMetaTransaction(const TypeNamespace&,
          << "    ::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 @@ bool HandleServerMetaTransaction(const TypeNamespace&, const AidlInterface& inte
   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 f017d626..8ced119f 100644
--- a/generate_java_binder.cpp
+++ b/generate_java_binder.cpp
@@ -828,6 +828,7 @@ static void generate_methods(const AidlInterface& iface, const AidlMethod& metho
            << "      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 ef8dbeda..24fee698 100644
--- a/tests/test_data_example_interface.cpp
+++ b/tests/test_data_example_interface.cpp
@@ -2518,6 +2518,7 @@ public interface IExampleInterface extends android.os.IInterface
           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 6dec57bf..9699d26c 100644
--- a/tests/test_data_ping_responder.cpp
+++ b/tests/test_data_ping_responder.cpp
@@ -690,9 +690,13 @@ int32_t BpPingResponder::getInterfaceVersion() {
     ::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 @@ namespace os {
     }
   }
   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 a83874c1..20778804 100644
--- a/tests/test_data_string_constants.cpp
+++ b/tests/test_data_string_constants.cpp
@@ -323,6 +323,7 @@ public interface IStringConstants extends android.os.IInterface
           try {
             data.writeInterfaceToken(DESCRIPTOR);
             mRemote.transact(Stub.TRANSACTION_getInterfaceVersion, data, reply, 0);
+            reply.readException();
             mCachedVersion = reply.readInt();
           } finally {
             reply.recycle();
@@ -429,9 +430,13 @@ int32_t BpStringConstants::getInterfaceVersion() {
     ::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 @@ namespace os {
 ::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;