Fix potential double close in IMediaMetadataRetriever::setDataSource

IMediaMetadataRetriever::setDataSource(fd, offset, length) takes the ownership
of |fd| on the direct invocation, and doesn't take the ownership on invocation
from Binder. This is inconsintent to other similar methods like
IMediaPlayer::setDataSource, and causes potential double close of |fd|.

This CL changes the caller and implementations to leave the ownership to make
them consistent.
Also, fixes a double close in IMediaPlayerService::setDataSource in an error
case.

Change-Id: Id551a1e725c4392b0fe6b7293871212eb101c0a5

2 files changed, 0 insertions, 4 deletions

diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index de51b3c..da5bb9a 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -749,7 +749,6 @@ status_t MediaPlayerService::Client::setDataSource(int fd, int64_t offset, int64
 
     if (offset >= sb.st_size) {
         ALOGE("offset error");
-        ::close(fd);
         return UNKNOWN_ERROR;
     }
     if (offset + length > sb.st_size) {
diff --git a/media/libmediaplayerservice/MetadataRetrieverClient.cpp b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
index 894a855..7bd99cc 100644
--- a/media/libmediaplayerservice/MetadataRetrieverClient.cpp
+++ b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
@@ -149,7 +149,6 @@ status_t MetadataRetrieverClient::setDataSource(int fd, int64_t offset, int64_t
 
     if (offset >= sb.st_size) {
         ALOGE("offset (%" PRId64 ") bigger than file size (%" PRIu64 ")", offset, sb.st_size);
-        ::close(fd);
         return BAD_VALUE;
     }
     if (offset + length > sb.st_size) {
@@ -165,12 +164,10 @@ status_t MetadataRetrieverClient::setDataSource(int fd, int64_t offset, int64_t
     ALOGV("player type = %d", playerType);
     sp<MediaMetadataRetrieverBase> p = createRetriever(playerType);
     if (p == NULL) {
-        ::close(fd);
         return NO_INIT;
     }
     status_t status = p->setDataSource(fd, offset, length);
     if (status == NO_ERROR) mRetriever = p;
-    ::close(fd);
     return status;
 }