MtpServer changes for MtpStorageManager
MoveObject and DeleteObject require
begin() and end() methods in order to
maintain database consistency. delete() now
has to return errors.
Allow sendObject after sendObjectInfo since
the spec allows this.
Test: See tests for main MtpStorageManager change
Bug: 63143623
Change-Id: Ied105e884cafd36e861521dcc59740e23b330f5f
diff --git a/media/mtp/MtpDatabase.h b/media/mtp/IMtpDatabase.h
similarity index 76%
rename from media/mtp/MtpDatabase.h
rename to media/mtp/IMtpDatabase.h
index f3f9720..d09a984 100644
--- a/media/mtp/MtpDatabase.h
+++ b/media/mtp/IMtpDatabase.h
@@ -14,8 +14,8 @@
* limitations under the License.
*/
-#ifndef _MTP_DATABASE_H
-#define _MTP_DATABASE_H
+#ifndef _I_MTP_DATABASE_H
+#define _I_MTP_DATABASE_H
#include "MtpTypes.h"
@@ -25,27 +25,24 @@
class MtpProperty;
class MtpObjectInfo;
-class MtpDatabase {
+class IMtpDatabase {
public:
- virtual ~MtpDatabase() {}
+ virtual ~IMtpDatabase() {}
- // called from SendObjectInfo to reserve a database entry for the incoming file
+ // Called from SendObjectInfo to reserve a database entry for the incoming file.
virtual MtpObjectHandle beginSendObject(const char* path,
MtpObjectFormat format,
MtpObjectHandle parent,
- MtpStorageID storage,
- uint64_t size,
- time_t modified) = 0;
+ MtpStorageID storage) = 0;
- // called to report success or failure of the SendObject file transfer
- // success should signal a notification of the new object's creation,
- // failure should remove the database entry created in beginSendObject
- virtual void endSendObject(const char* path,
- MtpObjectHandle handle,
- MtpObjectFormat format,
+ // Called to report success or failure of the SendObject file transfer.
+ virtual void endSendObject(MtpObjectHandle handle,
bool succeeded) = 0;
-
- virtual void doScanDirectory(const char* path) = 0;
+
+ // Called to rescan a file, such as after an edit.
+ virtual void rescanFile(const char* path,
+ MtpObjectHandle handle,
+ MtpObjectFormat format) = 0;
virtual MtpObjectHandleList* getObjectList(MtpStorageID storageID,
MtpObjectFormat format,
@@ -93,7 +90,8 @@
int64_t& outFileLength,
MtpObjectFormat& outFormat) = 0;
- virtual MtpResponseCode deleteFile(MtpObjectHandle handle) = 0;
+ virtual MtpResponseCode beginDeleteObject(MtpObjectHandle handle) = 0;
+ virtual void endDeleteObject(MtpObjectHandle handle, bool succeeded) = 0;
virtual MtpObjectHandleList* getObjectReferences(MtpObjectHandle handle) = 0;
@@ -105,14 +103,18 @@
virtual MtpProperty* getDevicePropertyDesc(MtpDeviceProperty property) = 0;
- virtual MtpResponseCode moveObject(MtpObjectHandle handle, MtpObjectHandle newParent,
- MtpStorageID newStorage, MtpString& newPath) = 0;
+ virtual MtpResponseCode beginMoveObject(MtpObjectHandle handle, MtpObjectHandle newParent,
+ MtpStorageID newStorage) = 0;
- virtual void sessionStarted() = 0;
+ virtual void endMoveObject(MtpObjectHandle oldParent, MtpObjectHandle newParent,
+ MtpStorageID oldStorage, MtpStorageID newStorage,
+ MtpObjectHandle handle, bool succeeded) = 0;
- virtual void sessionEnded() = 0;
+ virtual MtpResponseCode beginCopyObject(MtpObjectHandle handle, MtpObjectHandle newParent,
+ MtpStorageID newStorage);
+ virtual void endCopyObject(MtpObjectHandle handle, bool succeeded);
};
}; // namespace android
-#endif // _MTP_DATABASE_H
+#endif // _I_MTP_DATABASE_H
diff --git a/media/mtp/MtpServer.cpp b/media/mtp/MtpServer.cpp
index bb0414d..cfda0a6 100644
--- a/media/mtp/MtpServer.cpp
+++ b/media/mtp/MtpServer.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <algorithm>
#include <android-base/logging.h>
#include <android-base/properties.h>
#include <chrono>
@@ -31,7 +32,7 @@
#define LOG_TAG "MtpServer"
#include "MtpDebug.h"
-#include "MtpDatabase.h"
+#include "IMtpDatabase.h"
#include "MtpDevHandle.h"
#include "MtpFfsCompatHandle.h"
#include "MtpFfsHandle.h"
@@ -99,7 +100,7 @@
MTP_EVENT_DEVICE_PROP_CHANGED,
};
-MtpServer::MtpServer(MtpDatabase* database, bool ptp,
+MtpServer::MtpServer(IMtpDatabase* database, bool ptp,
const MtpString& deviceInfoManufacturer,
const MtpString& deviceInfoModel,
const MtpString& deviceInfoDeviceVersion,
@@ -150,21 +151,17 @@
void MtpServer::removeStorage(MtpStorage* storage) {
Mutex::Autolock autoLock(mMutex);
-
- for (size_t i = 0; i < mStorages.size(); i++) {
- if (mStorages[i] == storage) {
- mStorages.removeAt(i);
- sendStoreRemoved(storage->getStorageID());
- break;
- }
+ auto iter = std::find(mStorages.begin(), mStorages.end(), storage);
+ if (iter != mStorages.end()) {
+ sendStoreRemoved(storage->getStorageID());
+ mStorages.erase(iter);
}
}
MtpStorage* MtpServer::getStorage(MtpStorageID id) {
if (id == 0)
return mStorages[0];
- for (size_t i = 0; i < mStorages.size(); i++) {
- MtpStorage* storage = mStorages[i];
+ for (MtpStorage *storage : mStorages) {
if (storage->getStorageID() == id)
return storage;
}
@@ -265,9 +262,6 @@
}
mObjectEditList.clear();
- if (mSessionOpen)
- mDatabase->sessionEnded();
-
sHandle->close();
}
@@ -335,7 +329,7 @@
}
void MtpServer::commitEdit(ObjectEdit* edit) {
- mDatabase->endSendObject((const char *)edit->mPath, edit->mHandle, edit->mFormat, true);
+ mDatabase->rescanFile((const char *)edit->mPath, edit->mHandle, edit->mFormat);
}
@@ -348,9 +342,9 @@
mResponse.reset();
if (mSendObjectHandle != kInvalidObjectHandle && operation != MTP_OPERATION_SEND_OBJECT) {
- // FIXME - need to delete mSendObjectHandle from the database
- ALOGE("expected SendObject after SendObjectInfo");
mSendObjectHandle = kInvalidObjectHandle;
+ mSendObjectFormat = 0;
+ mSendObjectModifiedTime = 0;
}
int containertype = mRequest.getContainerType();
@@ -526,8 +520,6 @@
mSessionID = mRequest.getParameter(1);
mSessionOpen = true;
- mDatabase->sessionStarted();
-
return MTP_RESPONSE_OK;
}
@@ -536,7 +528,6 @@
return MTP_RESPONSE_SESSION_NOT_OPEN;
mSessionID = 0;
mSessionOpen = false;
- mDatabase->sessionEnded();
return MTP_RESPONSE_OK;
}
@@ -604,6 +595,8 @@
return MTP_RESPONSE_INVALID_STORAGE_ID;
MtpObjectHandleList* handles = mDatabase->getObjectList(storageID, format, parent);
+ if (handles == NULL)
+ return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
mData.putAUInt32(handles);
delete handles;
return MTP_RESPONSE_OK;
@@ -991,26 +984,25 @@
}
ALOGD("path: %s parent: %d storageID: %08X", (const char*)path, parent, storageID);
- MtpObjectHandle handle = mDatabase->beginSendObject((const char*)path,
- format, parent, storageID, mSendObjectFileSize, modifiedTime);
+ MtpObjectHandle handle = mDatabase->beginSendObject((const char*)path, format,
+ parent, storageID);
if (handle == kInvalidObjectHandle) {
return MTP_RESPONSE_GENERAL_ERROR;
}
- if (format == MTP_FORMAT_ASSOCIATION) {
+ if (format == MTP_FORMAT_ASSOCIATION) {
int ret = makeFolder((const char *)path);
if (ret)
return MTP_RESPONSE_GENERAL_ERROR;
// SendObject does not get sent for directories, so call endSendObject here instead
- mDatabase->endSendObject(path, handle, MTP_FORMAT_ASSOCIATION, MTP_RESPONSE_OK);
- } else {
- mSendObjectFilePath = path;
- // save the handle for the SendObject call, which should follow
- mSendObjectHandle = handle;
- mSendObjectFormat = format;
- mSendObjectModifiedTime = modifiedTime;
+ mDatabase->endSendObject(handle, MTP_RESPONSE_OK);
}
+ mSendObjectFilePath = path;
+ // save the handle for the SendObject call, which should follow
+ mSendObjectHandle = handle;
+ mSendObjectFormat = format;
+ mSendObjectModifiedTime = modifiedTime;
mResponse.setParameter(1, storageID);
mResponse.setParameter(2, parent);
@@ -1061,6 +1053,10 @@
path += "/";
path += info.mName;
+ result = mDatabase->beginMoveObject(objectHandle, parent, storageID);
+ if (result != MTP_RESPONSE_OK)
+ return result;
+
if (info.mStorageID == storageID) {
ALOGV("Moving file from %s to %s", (const char*)fromPath, (const char*)path);
if (rename(fromPath, path)) {
@@ -1087,8 +1083,8 @@
}
// If the move failed, undo the database change
- if (result == MTP_RESPONSE_OK)
- result = mDatabase->moveObject(objectHandle, parent, storageID, path);
+ mDatabase->endMoveObject(info.mParent, parent, info.mStorageID, storageID, objectHandle,
+ result == MTP_RESPONSE_OK);
return result;
}
@@ -1139,8 +1135,7 @@
path += "/";
path += info.mName;
- MtpObjectHandle handle = mDatabase->beginSendObject((const char*)path,
- format, parent, storageID, fileLength, info.mDateModified);
+ MtpObjectHandle handle = mDatabase->beginCopyObject(objectHandle, parent, storageID);
if (handle == kInvalidObjectHandle) {
return MTP_RESPONSE_GENERAL_ERROR;
}
@@ -1158,9 +1153,7 @@
}
}
- mDatabase->endSendObject(path, handle, format, result);
- if (format == MTP_FORMAT_ASSOCIATION)
- mDatabase->doScanDirectory(path);
+ mDatabase->endCopyObject(handle, result);
mResponse.setParameter(1, handle);
return result;
}
@@ -1190,6 +1183,15 @@
}
initialData = ret - MTP_CONTAINER_HEADER_SIZE;
+ if (mSendObjectFormat == MTP_FORMAT_ASSOCIATION) {
+ if (initialData != 0)
+ ALOGE("Expected folder size to be 0!");
+ mSendObjectHandle = kInvalidObjectHandle;
+ mSendObjectFormat = 0;
+ mSendObjectModifiedTime = 0;
+ return result;
+ }
+
mtp_file_range mfr;
mfr.fd = open(mSendObjectFilePath, O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
if (mfr.fd < 0) {
@@ -1255,8 +1257,7 @@
// reset so we don't attempt to send the data back
mData.reset();
- mDatabase->endSendObject(mSendObjectFilePath, mSendObjectHandle, mSendObjectFormat,
- result == MTP_RESPONSE_OK);
+ mDatabase->endSendObject(mSendObjectHandle, result == MTP_RESPONSE_OK);
mSendObjectHandle = kInvalidObjectHandle;
mSendObjectFormat = 0;
mSendObjectModifiedTime = 0;
@@ -1282,16 +1283,18 @@
MtpString filePath;
int64_t fileLength;
int result = mDatabase->getObjectFilePath(handle, filePath, fileLength, format);
- if (result == MTP_RESPONSE_OK) {
- ALOGV("deleting %s", (const char *)filePath);
- result = mDatabase->deleteFile(handle);
- // Don't delete the actual files unless the database deletion is allowed
- if (result == MTP_RESPONSE_OK) {
- deletePath((const char *)filePath);
- }
- }
+ if (result != MTP_RESPONSE_OK)
+ return result;
- return result;
+ // Don't delete the actual files unless the database deletion is allowed
+ result = mDatabase->beginDeleteObject(handle);
+ if (result != MTP_RESPONSE_OK)
+ return result;
+
+ bool success = deletePath((const char *)filePath);
+
+ mDatabase->endDeleteObject(handle, success);
+ return success ? result : MTP_RESPONSE_PARTIAL_DELETION;
}
MtpResponseCode MtpServer::doGetObjectPropDesc() {
diff --git a/media/mtp/MtpServer.h b/media/mtp/MtpServer.h
index 0204b09..af371eb 100644
--- a/media/mtp/MtpServer.h
+++ b/media/mtp/MtpServer.h
@@ -32,13 +32,13 @@
namespace android {
-class MtpDatabase;
+class IMtpDatabase;
class MtpStorage;
class MtpServer {
private:
- MtpDatabase* mDatabase;
+ IMtpDatabase* mDatabase;
// appear as a PTP device
bool mPtp;
@@ -98,7 +98,7 @@
Vector<ObjectEdit*> mObjectEditList;
public:
- MtpServer(MtpDatabase* database, bool ptp,
+ MtpServer(IMtpDatabase* database, bool ptp,
const MtpString& deviceInfoManufacturer,
const MtpString& deviceInfoModel,
const MtpString& deviceInfoDeviceVersion,
diff --git a/media/mtp/MtpStorage.cpp b/media/mtp/MtpStorage.cpp
index d77ca72..a147325 100644
--- a/media/mtp/MtpStorage.cpp
+++ b/media/mtp/MtpStorage.cpp
@@ -17,7 +17,6 @@
#define LOG_TAG "MtpStorage"
#include "MtpDebug.h"
-#include "MtpDatabase.h"
#include "MtpStorage.h"
#include <sys/types.h>
@@ -33,14 +32,12 @@
namespace android {
MtpStorage::MtpStorage(MtpStorageID id, const char* filePath,
- const char* description, uint64_t reserveSpace,
- bool removable, uint64_t maxFileSize)
+ const char* description, bool removable, uint64_t maxFileSize)
: mStorageID(id),
mFilePath(filePath),
mDescription(description),
mMaxCapacity(0),
mMaxFileSize(maxFileSize),
- mReserveSpace(reserveSpace),
mRemovable(removable)
{
ALOGV("MtpStorage id: %d path: %s\n", id, filePath);
@@ -75,8 +72,7 @@
struct statfs stat;
if (statfs(getPath(), &stat))
return -1;
- uint64_t freeSpace = (uint64_t)stat.f_bavail * (uint64_t)stat.f_bsize;
- return (freeSpace > mReserveSpace ? freeSpace - mReserveSpace : 0);
+ return (uint64_t)stat.f_bavail * (uint64_t)stat.f_bsize;
}
const char* MtpStorage::getDescription() const {
diff --git a/media/mtp/MtpStorage.h b/media/mtp/MtpStorage.h
index e5a2e57..cb7e333 100644
--- a/media/mtp/MtpStorage.h
+++ b/media/mtp/MtpStorage.h
@@ -32,13 +32,11 @@
MtpString mDescription;
uint64_t mMaxCapacity;
uint64_t mMaxFileSize;
- // amount of free space to leave unallocated
- uint64_t mReserveSpace;
bool mRemovable;
public:
MtpStorage(MtpStorageID id, const char* filePath,
- const char* description, uint64_t reserveSpace,
+ const char* description,
bool removable, uint64_t maxFileSize);
virtual ~MtpStorage();
diff --git a/media/mtp/MtpUtils.cpp b/media/mtp/MtpUtils.cpp
index 3f5648b..51cfd7d 100644
--- a/media/mtp/MtpUtils.cpp
+++ b/media/mtp/MtpUtils.cpp
@@ -204,29 +204,39 @@
if (name[0] == '.' && (name[1] == 0 || (name[1] == '.' && name[2] == 0))) {
continue;
}
- pathStr.append(name);
+ string childPath = pathStr + name;
+ int success;
if (entry->d_type == DT_DIR) {
- deleteRecursive(pathStr.c_str());
- rmdir(pathStr.c_str());
+ deleteRecursive(childPath.c_str());
+ success = rmdir(childPath.c_str());
} else {
- unlink(pathStr.c_str());
+ success = unlink(childPath.c_str());
}
+ if (success == -1)
+ PLOG(ERROR) << "Deleting path " << childPath << " failed";
}
closedir(dir);
}
-void deletePath(const char* path) {
+bool deletePath(const char* path) {
struct stat statbuf;
+ int success;
if (stat(path, &statbuf) == 0) {
if (S_ISDIR(statbuf.st_mode)) {
+ // rmdir will fail if the directory is non empty, so
+ // there is no need to keep errors from deleteRecursive
deleteRecursive(path);
- rmdir(path);
+ success = rmdir(path);
} else {
- unlink(path);
+ success = unlink(path);
}
} else {
- PLOG(ERROR) << "deletePath stat failed for " << path;;
+ PLOG(ERROR) << "deletePath stat failed for " << path;
+ return false;
}
+ if (success == -1)
+ PLOG(ERROR) << "Deleting path " << path << " failed";
+ return success == 0;
}
} // namespace android
diff --git a/media/mtp/MtpUtils.h b/media/mtp/MtpUtils.h
index b7c72f5..744546b 100644
--- a/media/mtp/MtpUtils.h
+++ b/media/mtp/MtpUtils.h
@@ -33,8 +33,7 @@
int makeFolder(const char *path);
int copyRecursive(const char *fromPath, const char *toPath);
int copyFile(const char *fromPath, const char *toPath);
-void deleteRecursive(const char* path);
-void deletePath(const char* path);
+bool deletePath(const char* path);
}; // namespace android