Merge changes I0ef88864,I7b892a22

* changes:
  binder: send BC_REPLY and BC_FREE_BUFFER together
  binder: remove unnecessary err check

2 files changed, 66 insertions, 69 deletions

diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index 1c3fab4057..50974ef74f 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -571,7 +571,7 @@ status_t IPCThreadState::transact(int32_t handle,
                                   uint32_t code, const Parcel& data,
                                   Parcel* reply, uint32_t flags)
 {
-    status_t err = data.errorCheck();
+    status_t err;
 
     flags |= TF_ACCEPT_FDS;
 
@@ -582,11 +582,9 @@ status_t IPCThreadState::transact(int32_t handle,
             << indent << data << dedent << endl;
     }
 
-    if (err == NO_ERROR) {
-        LOG_ONEWAY(">>>> SEND from pid %d uid %d %s", getpid(), getuid(),
-            (flags & TF_ONE_WAY) == 0 ? "READ REPLY" : "ONE WAY");
-        err = writeTransactionData(BC_TRANSACTION, flags, handle, code, data, NULL);
-    }
+    LOG_ONEWAY(">>>> SEND from pid %d uid %d %s", getpid(), getuid(),
+        (flags & TF_ONE_WAY) == 0 ? "READ REPLY" : "ONE WAY");
+    err = writeTransactionData(BC_TRANSACTION, flags, handle, code, data, NULL);
 
     if (err != NO_ERROR) {
         if (reply) reply->setError(err);
@@ -720,16 +718,6 @@ IPCThreadState::~IPCThreadState()
 {
 }
 
-status_t IPCThreadState::sendReply(const Parcel& reply, uint32_t flags)
-{
-    status_t err;
-    status_t statusBuffer;
-    err = writeTransactionData(BC_REPLY, flags, -1, 0, reply, &statusBuffer);
-    if (err < NO_ERROR) return err;
-
-    return waitForResponse(NULL, NULL);
-}
-
 status_t IPCThreadState::waitForResponse(Parcel *reply, status_t *acquireResult)
 {
     uint32_t cmd;
@@ -1052,68 +1040,78 @@ status_t IPCThreadState::executeCommand(int32_t cmd)
                 "Not enough command data for brTRANSACTION");
             if (result != NO_ERROR) break;
 
-            Parcel buffer;
-            buffer.ipcSetDataReference(
-                reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),
-                tr.data_size,
-                reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),
-                tr.offsets_size/sizeof(binder_size_t), freeBuffer, this);
+            Parcel reply;
+            {
+                Parcel buffer;
+                const pid_t origPid = mCallingPid;
+                const uid_t origUid = mCallingUid;
+                const int32_t origStrictModePolicy = mStrictModePolicy;
+                const int32_t origTransactionBinderFlags = mLastTransactionBinderFlags;
+
+                buffer.ipcSetDataReference(
+                    reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),
+                    tr.data_size,
+                    reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),
+                    tr.offsets_size/sizeof(binder_size_t), freeBuffer, this);
+
+                mCallingPid = tr.sender_pid;
+                mCallingUid = tr.sender_euid;
+                mLastTransactionBinderFlags = tr.flags;
+
+                //ALOGI(">>>> TRANSACT from pid %d uid %d\n", mCallingPid, mCallingUid);
+
+                status_t error;
+                IF_LOG_TRANSACTIONS() {
+                    TextOutput::Bundle _b(alog);
+                    alog << "BR_TRANSACTION thr " << (void*)pthread_self()
+                        << " / obj " << tr.target.ptr << " / code "
+                        << TypeCode(tr.code) << ": " << indent << buffer
+                        << dedent << endl
+                        << "Data addr = "
+                        << reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer)
+                        << ", offsets addr="
+                        << reinterpret_cast<const size_t*>(tr.data.ptr.offsets) << endl;
+                }
+                if (tr.target.ptr) {
+                    // We only have a weak reference on the target object, so we must first try to
+                    // safely acquire a strong reference before doing anything else with it.
+                    if (reinterpret_cast<RefBase::weakref_type*>(
+                            tr.target.ptr)->attemptIncStrong(this)) {
+                        error = reinterpret_cast<BBinder*>(tr.cookie)->transact(tr.code, buffer,
+                                &reply, tr.flags);
+                        reinterpret_cast<BBinder*>(tr.cookie)->decStrong(this);
+                    } else {
+                        error = UNKNOWN_TRANSACTION;
+                    }
 
-            const pid_t origPid = mCallingPid;
-            const uid_t origUid = mCallingUid;
-            const int32_t origStrictModePolicy = mStrictModePolicy;
-            const int32_t origTransactionBinderFlags = mLastTransactionBinderFlags;
+                } else {
+                    error = the_context_object->transact(tr.code, buffer, &reply, tr.flags);
+                }
 
-            mCallingPid = tr.sender_pid;
-            mCallingUid = tr.sender_euid;
-            mLastTransactionBinderFlags = tr.flags;
+                //ALOGI("<<<< TRANSACT from pid %d restore pid %d uid %d\n",
+                //     mCallingPid, origPid, origUid);
 
-            //ALOGI(">>>> TRANSACT from pid %d uid %d\n", mCallingPid, mCallingUid);
+                if ((tr.flags & TF_ONE_WAY) == 0) {
+                    LOG_ONEWAY("Sending reply to %d!", mCallingPid);
+                    if (error < NO_ERROR) reply.setError(error);
 
-            Parcel reply;
-            status_t error;
-            IF_LOG_TRANSACTIONS() {
-                TextOutput::Bundle _b(alog);
-                alog << "BR_TRANSACTION thr " << (void*)pthread_self()
-                    << " / obj " << tr.target.ptr << " / code "
-                    << TypeCode(tr.code) << ": " << indent << buffer
-                    << dedent << endl
-                    << "Data addr = "
-                    << reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer)
-                    << ", offsets addr="
-                    << reinterpret_cast<const size_t*>(tr.data.ptr.offsets) << endl;
-            }
-            if (tr.target.ptr) {
-                // We only have a weak reference on the target object, so we must first try to
-                // safely acquire a strong reference before doing anything else with it.
-                if (reinterpret_cast<RefBase::weakref_type*>(
-                        tr.target.ptr)->attemptIncStrong(this)) {
-                    error = reinterpret_cast<BBinder*>(tr.cookie)->transact(tr.code, buffer,
-                            &reply, tr.flags);
-                    reinterpret_cast<BBinder*>(tr.cookie)->decStrong(this);
+                    status_t statusBuffer;
+                    result = writeTransactionData(BC_REPLY, 0, -1, 0, reply, &statusBuffer);
                 } else {
-                    error = UNKNOWN_TRANSACTION;
+                    LOG_ONEWAY("NOT sending reply to %d!", mCallingPid);
                 }
 
-            } else {
-                error = the_context_object->transact(tr.code, buffer, &reply, tr.flags);
+                mCallingPid = origPid;
+                mCallingUid = origUid;
+                mStrictModePolicy = origStrictModePolicy;
+                mLastTransactionBinderFlags = origTransactionBinderFlags;
             }
 
-            //ALOGI("<<<< TRANSACT from pid %d restore pid %d uid %d\n",
-            //     mCallingPid, origPid, origUid);
-
-            if ((tr.flags & TF_ONE_WAY) == 0) {
-                LOG_ONEWAY("Sending reply to %d!", mCallingPid);
-                if (error < NO_ERROR) reply.setError(error);
-                sendReply(reply, 0);
-            } else {
-                LOG_ONEWAY("NOT sending reply to %d!", mCallingPid);
-            }
+            if (result != NO_ERROR)
+                break;
 
-            mCallingPid = origPid;
-            mCallingUid = origUid;
-            mStrictModePolicy = origStrictModePolicy;
-            mLastTransactionBinderFlags = origTransactionBinderFlags;
+            if ((tr.flags & TF_ONE_WAY) == 0)
+                waitForResponse(NULL, NULL);
 
             IF_LOG_TRANSACTIONS() {
                 TextOutput::Bundle _b(alog);
diff --git a/libs/binder/include/binder/IPCThreadState.h b/libs/binder/include/binder/IPCThreadState.h
index 245607e74e..9fb13bbe95 100644
--- a/libs/binder/include/binder/IPCThreadState.h
+++ b/libs/binder/include/binder/IPCThreadState.h
@@ -93,7 +93,6 @@ private:
                                 IPCThreadState();
                                 ~IPCThreadState();
 
-            status_t            sendReply(const Parcel& reply, uint32_t flags);
             status_t            waitForResponse(Parcel *reply,
                                                 status_t *acquireResult=NULL);
             status_t            talkWithDriver(bool doReceive=true);