commit 81ca5ade77ea518aa21fdd7434d2db5df088a115
author Daichi Hirono <hirono@google.com> Tue Aug 18 21:13:40 2015 +0900
committer Daichi Hirono <hirono@google.com> Thu Aug 20 15:14:53 2015 +0900
tree f201610ca81c7731190a11ca884a8678415f6245
parent 2b355c2063fa89a50c5fd463c00a47633dfee56b

Read whole data from MTP device even on writing error.

Once the object bytes is requested on the MTP client device, the device tries to
send whole data of object. We need to read the complete data from the device
even when we have errors at the destination file descriptor. Otherwise the
object data will be received as a response of next request unintentionally.

BUG=23264575

Change-Id: I3369786370022f65aa760dd6b75204a946f712af

media/mtp/MtpDevice.cpp

diff --git a/media/mtp/MtpDevice.cpp b/media/mtp/MtpDevice.cpp
index d54ce50..b84a245 100644
--- a/media/mtp/MtpDevice.cpp
+++ b/media/mtp/MtpDevice.cpp
@@ -683,7 +683,6 @@
     return result;
 }
 
-
 // reads the object's data and writes it to the specified file path
 bool MtpDevice::readObject(MtpObjectHandle handle, const char* destPath, int group, int perm) {
     ALOGD("readObject: %s", destPath);
@@ -708,25 +707,26 @@
     ALOGD("readObject: %d", fd);
 
     Mutex::Autolock autoLock(mMutex);
-    bool result = false;
 
     mRequest.reset();
     mRequest.setParameter(1, handle);
-    if (sendRequest(MTP_OPERATION_GET_OBJECT)
-            && mData.readDataHeader(mRequestIn1)) {
+    if (sendRequest(MTP_OPERATION_GET_OBJECT) && mData.readDataHeader(mRequestIn1)) {
         uint32_t length = mData.getContainerLength();
-        if (length < MTP_CONTAINER_HEADER_SIZE)
-            goto fail;
+        if (length < MTP_CONTAINER_HEADER_SIZE) {
+            ALOGE("Invalid container length.");
+            return false;
+        }
         length -= MTP_CONTAINER_HEADER_SIZE;
         uint32_t remaining = length;
 
+        bool writingError = false;
         int initialDataLength = 0;
         void* initialData = mData.getData(initialDataLength);
         if (initialData) {
             if (initialDataLength > 0) {
                 if (write(fd, initialData, initialDataLength) != initialDataLength) {
-                    free(initialData);
-                    goto fail;
+                    ALOGE("Failed to write initial data.");
+                    writingError = true;
                 }
                 remaining -= initialDataLength;
             }
@@ -747,29 +747,27 @@
                 req->buffer_length = (remaining > sizeof(buffer1) ? sizeof(buffer1) : remaining);
                 if (mData.readDataAsync(req)) {
                     ALOGE("readDataAsync failed");
-                    goto fail;
+                    return false;
                 }
             } else {
                 req = NULL;
             }
 
-            if (writeBuffer) {
+            if (writeBuffer && !writingError) {
                 // write previous buffer
                 if (write(fd, writeBuffer, writeLength) != writeLength) {
-                    ALOGE("write failed");
-                    // wait for pending read before failing
-                    if (req)
-                        mData.readDataWait(mDevice);
-                    goto fail;
+                    writingError = true;
                 }
-                writeBuffer = NULL;
             }
+            writeBuffer = NULL;
 
             // wait for read to complete
             if (req) {
                 int read = mData.readDataWait(mDevice);
-                if (read < 0)
-                    goto fail;
+                if (read < 0) {
+                    ALOGE("readDataWait failed.");
+                    return false;
+                }
 
                 if (read > 0) {
                     writeBuffer = req->buffer;
@@ -783,12 +781,10 @@
         }
 
         MtpResponseCode response = readResponse();
-        if (response == MTP_RESPONSE_OK)
-            result = true;
+        return response == MTP_RESPONSE_OK && !writingError;
     }
 
-fail:
-    return result;
+    return false;
 }
 
 bool MtpDevice::sendRequest(MtpOperationCode operation) {