From beb0b4e9f30084bc4cbbb3c8e1fa1f81b8a247bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 24 Apr 2021 18:36:25 +0200 Subject: [PATCH] More work on AndroidStorageFileSystem.cpp. Fix current directory getting reset. --- CMakeLists.txt | 2 + Core/Config.cpp | 6 +- Core/FileSystems/AndroidStorageFileSystem.cpp | 136 +++++++++--------- Core/FileSystems/AndroidStorageFileSystem.h | 1 - Core/FileSystems/DirectoryFileSystem.cpp | 2 +- android/jni/app-android.cpp | 20 ++- android/jni/app-android.h | 13 +- .../src/org/ppsspp/ppsspp/PpssppActivity.java | 4 +- 8 files changed, 102 insertions(+), 82 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fb6cba591a..fa196f6d23 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1621,6 +1621,8 @@ add_library(${CoreLibName} ${CoreLinkType} Core/ELF/ParamSFO.cpp Core/ELF/ParamSFO.h Core/FileSystems/tlzrc.cpp + Core/FileSystems/AndroidStorageFileSystem.cpp + Core/FileSystems/AndroidStorageFileSystem.h Core/FileSystems/BlobFileSystem.cpp Core/FileSystems/BlobFileSystem.h Core/FileSystems/BlockDevices.cpp diff --git a/Core/Config.cpp b/Core/Config.cpp index 74be99d75e..3a461dc8d2 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -1240,7 +1240,11 @@ void Config::Load(const char *iniFileName, const char *controllerIniFilename) { }); iRunCount++; - if (!File::Exists(Path(currentDirectory))) + + // TODO: What layer's responsibility is it to handle content:// URIs? + // This check is probably not really necessary here anyway, you can always + // press Home or Browse if you're in a bad directory. + if (!File::Exists(currentDirectory)) currentDirectory = defaultCurrentDirectory; Section *log = iniFile.GetOrCreateSection(logSectionName); diff --git a/Core/FileSystems/AndroidStorageFileSystem.cpp b/Core/FileSystems/AndroidStorageFileSystem.cpp index 60e2663696..2c7878e0c3 100644 --- a/Core/FileSystems/AndroidStorageFileSystem.cpp +++ b/Core/FileSystems/AndroidStorageFileSystem.cpp @@ -19,7 +19,7 @@ #if PPSSPP_PLATFORM(ANDROID) #include "android/jni/app-android.h" -#endif +#include "android/jni/AndroidContentURI.h" #include #include @@ -52,6 +52,8 @@ AndroidStorageFileSystem::~AndroidStorageFileSystem() { bool AndroidDirectoryFileHandle::Open(const std::string &basePath, std::string &fileName, FileAccess access, u32 &error) { error = 0; + AndroidStorageContentURI contentUri = AndroidStorageContentURI(basePath).WithFilePath(fileName); + // On the PSP, truncating doesn't lose data. If you seek later, you'll recover it. // This is abnormal, so we deviate from the PSP's behavior and truncate on write/close. // This means it's incorrectly not truncated before the write. @@ -59,81 +61,77 @@ bool AndroidDirectoryFileHandle::Open(const std::string &basePath, std::string & needsTrunc_ = 0; } - //TODO: tests, should append seek to end of file? seeking in a file opened for append? - - int flags = 0; - if (access & FILEACCESS_APPEND) { - flags |= O_APPEND; + // Check for existance and directory, so we can make better decisions below. + File::FileInfo fileInfo; + if (!Android_GetFileInfo(contentUri.ToString(), &fileInfo)) { + ERROR_LOG(FILESYS, "Failed to call Android_GetFileInfo on %s", contentUri.ToString().c_str()); + return false; } - if ((access & FILEACCESS_READ) && (access & FILEACCESS_WRITE)) { - flags |= O_RDWR; + + if (fileInfo.exists && fileInfo.isDirectory) { + ERROR_LOG(FILESYS, "AndroidDirectoryFileHandle::Open on directory %s - not allowed", contentUri.ToString().c_str()); + return false; + } + + // Android_OpenContentUriFd only has three modes: READ/READ_WRITE/READ_WRITE_TRUNCATE. + // For the modes that can create files, we have to explicitly create the file first, instead. + // My god, what a pain this is. Gonna need some sort of test suite. + + Android_OpenContentUriMode mode = Android_OpenContentUriMode::READ; + + // TODO: There are probably improvements to be made in this translation. + if (access & FILEACCESS_WRITE) { + mode = Android_OpenContentUriMode::READ_WRITE_TRUNCATE; + // Should this require EXCL too? + if (access & FILEACCESS_APPEND) { + mode = Android_OpenContentUriMode::READ_WRITE; + // Will also need to seek later. + } } else if (access & FILEACCESS_READ) { - flags |= O_RDONLY; - } else if (access & FILEACCESS_WRITE) { - flags |= O_WRONLY; + mode = Android_OpenContentUriMode::READ; } + + bool success = true; + if (access & FILEACCESS_CREATE) { - flags |= O_CREAT; - } - if (access & FILEACCESS_EXCL) { - flags |= O_EXCL; - } + AndroidStorageContentURI parentUri(basePath); - hFile = open(fullName.c_str(), flags, 0666); - bool success = hFile != -1; - -#if HOST_IS_CASE_SENSITIVE - if (!success && !(access & FILEACCESS_CREATE)) { - if (!FixPathCase(basePath, fileName, FPC_PATH_MUST_EXIST)) { + // We're gonna have to explicitly create the file here. + if (!Android_CreateFile(parentUri.ToString(), fileName)) { + // Something went wrong. + success = false; + } + } else { + // Requested an existing file, doesn't exist. + if (!fileInfo.exists) { error = SCE_KERNEL_ERROR_ERRNO_FILE_NOT_FOUND; return false; } - fullName = GetLocalPath(basePath, fileName); - const char *fullNameC = fullName.c_str(); - - DEBUG_LOG(FILESYS, "Case may have been incorrect, second try opening %s (%s)", fullNameC, fileName.c_str()); - - // And try again with the correct case this time -#ifdef _WIN32 - hFile = CreateFile(fullNameC, desired, sharemode, 0, openmode, 0, 0); - success = hFile != INVALID_HANDLE_VALUE; -#else - hFile = open(fullNameC, flags, 0666); - success = hFile != -1; -#endif } -#endif -#ifndef _WIN32 - if (success) { - // Reject directories, even if we succeed in opening them. - // TODO: Might want to do this stat first... - struct stat st; - if (fstat(hFile, &st) == 0 && S_ISDIR(st.st_mode)) { - close(hFile); - errno = EISDIR; - success = false; - } - } else if (errno == ENOSPC) { - // This is returned when the disk is full. - auto err = GetI18NCategory("Error"); - host->NotifyUserMessage(err->T("Disk full while writing data")); - error = SCE_KERNEL_ERROR_ERRNO_NO_PERM; - } else { - error = SCE_KERNEL_ERROR_ERRNO_FILE_NOT_FOUND; + // Not sure what to do with FILEACCESS_EXCL. + + std::string uriString = contentUri.ToString(); + + INFO_LOG(FILESYS, "AndroidDirectoryFileHandle::Open(%s)", uriString.c_str()); + + hFile = Android_OpenContentUriFd(uriString, mode); + if (hFile == -1) { + ERROR_LOG(FILESYS, "Android_OpenContentUriFd(%s) failed", uriString.c_str()); + success = false; } -#endif + + // Seek to end if append mode. + Seek(0, FILEMOVE_END); // Try to detect reads/writes to PSP/GAME to avoid them in replays. - if (fullName.find("/PSP/GAME/") != fullName.npos || fullName.find("\\PSP\\GAME\\") != fullName.npos) { + if (basePath.find("/PSP/GAME/") != std::string::npos) { inGameDir_ = true; } - return success; } -size_t AndroidDirectoryFileHandle::Read(u8* pointer, s64 size) -{ +size_t AndroidDirectoryFileHandle::Read(u8* pointer, s64 size) { size_t bytesRead = 0; if (needsTrunc_ != -1) { // If the file was marked to be truncated, pretend there's nothing. @@ -400,7 +398,7 @@ PSPFileInfo AndroidStorageFileSystem::GetFileInfo(std::string filename) { std::string uri = GetLocalPath(filename); - FileInfo info; + File::FileInfo info; if (!Android_GetFileInfo(uri, &info)) { return ReplayApplyDiskFileInfo(x, CoreTiming::GetGlobalTimeUs()); } @@ -427,11 +425,6 @@ PSPFileInfo AndroidStorageFileSystem::GetFileInfo(std::string filename) { return ReplayApplyDiskFileInfo(x, CoreTiming::GetGlobalTimeUs()); } -bool AndroidStorageFileSystem::GetHostPath(const std::string &inpath, std::string &outpath) { - outpath = GetLocalPath(inpath); - return true; -} - // See comment in DirectoryFileSystem. extern std::string SimulateVFATBug(std::string filename); @@ -441,7 +434,7 @@ std::vector AndroidStorageFileSystem::GetDirListing(std::string pat std::string uri = GetLocalPath(path); - std::vector fileInfo = Android_ListContentUri(uri); + std::vector fileInfo = Android_ListContentUri(uri); bool hideISOFiles = PSP_CoreParameter().compat.flags().HideISOFiles; for (auto &info : fileInfo) { @@ -452,8 +445,9 @@ std::vector AndroidStorageFileSystem::GetDirListing(std::string pat entry.type = FILETYPE_NORMAL; entry.access = info.isWritable ? 0777 : 0666; entry.name = info.name; - if (Flags() & FileSystemFlags::SIMULATE_FAT32) + if (Flags() & FileSystemFlags::SIMULATE_FAT32) { entry.name = SimulateVFATBug(entry.name); + } entry.size = info.size; bool hideFile = false; @@ -461,15 +455,17 @@ std::vector AndroidStorageFileSystem::GetDirListing(std::string pat // Workaround for DJ Max Portable, see compat.ini. hideFile = true; } + + // TODO: Maybe do this conversion in the Android wrapper layer... int64_t lastModified = info.lastModified / 1000; time_t atime = lastModified; time_t ctime = lastModified; time_t mtime = lastModified; - localtime_r((time_t*)&s.st_atime, &entry.atime); - localtime_r((time_t*)&s.st_ctime, &entry.ctime); - localtime_r((time_t*)&s.st_mtime, &entry.mtime); + localtime_r((time_t*)&atime, &entry.atime); + localtime_r((time_t*)&ctime, &entry.ctime); + localtime_r((time_t*)&mtime, &entry.mtime); if (!hideFile && (!listingRoot || (strcmp(info.name.c_str(), "..") && strcmp(info.name.c_str(), ".")))) myVector.push_back(entry); } @@ -537,3 +533,5 @@ void AndroidStorageFileSystem::DoState(PointerWrap &p) { } } } + +#endif // PPSSPP_PLATFORM(ANDROID) diff --git a/Core/FileSystems/AndroidStorageFileSystem.h b/Core/FileSystems/AndroidStorageFileSystem.h index 25fceff1dd..01af2bb318 100644 --- a/Core/FileSystems/AndroidStorageFileSystem.h +++ b/Core/FileSystems/AndroidStorageFileSystem.h @@ -79,7 +79,6 @@ public: bool RmDir(const std::string &dirname) override; int RenameFile(const std::string &from, const std::string &to) override; bool RemoveFile(const std::string &filename) override; - bool GetHostPath(const std::string &inpath, std::string &outpath) override; FileSystemFlags Flags() override { return flags; } u64 FreeSpace(const std::string &path) override; diff --git a/Core/FileSystems/DirectoryFileSystem.cpp b/Core/FileSystems/DirectoryFileSystem.cpp index 5e84837b31..d15c953408 100644 --- a/Core/FileSystems/DirectoryFileSystem.cpp +++ b/Core/FileSystems/DirectoryFileSystem.cpp @@ -787,7 +787,7 @@ static void tmFromFiletime(tm &dest, FILETIME &src) { // // Note: PSP-created files would stay lowercase, but this uppercases them too. // Hopefully no PSP games read directories after they create files in them... -static std::string SimulateVFATBug(std::string filename) { +std::string SimulateVFATBug(std::string filename) { // These are the characters allowed in DOS filenames. static const char *FAT_UPPER_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&'(){}-_`~"; static const char *FAT_LOWER_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789!@#$%^&'(){}-_`~"; diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 6498c348f4..f0338cf0e4 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -241,7 +241,7 @@ bool Android_IsContentUri(const std::string &filename) { return startsWith(filename, "content://"); } -int Android_OpenContentUriFd(const std::string &filename) { +int Android_OpenContentUriFd(const std::string &filename, Android_OpenContentUriMode mode) { if (!nativeActivity) { return -1; } @@ -251,9 +251,17 @@ int Android_OpenContentUriFd(const std::string &filename) { // TODO: Fix that in the caller (or don't call this for directories). if (fname.back() == '/') fname.pop_back(); + auto env = getEnv(); - jstring param = env->NewStringUTF(fname.c_str()); - int fd = env->CallIntMethod(nativeActivity, openContentUri, param); + const char *modeStr = ""; + switch (mode) { + case Android_OpenContentUriMode::READ: modeStr = "r"; break; + case Android_OpenContentUriMode::READ_WRITE: modeStr = "rw"; break; + case Android_OpenContentUriMode::READ_WRITE_TRUNCATE: modeStr = "rwt"; break; + } + jstring j_filename = env->NewStringUTF(fname.c_str()); + jstring j_mode = env->NewStringUTF(modeStr); + int fd = env->CallIntMethod(nativeActivity, openContentUri, j_filename, j_mode); return fd; } @@ -286,7 +294,7 @@ bool Android_RemoveFile(const std::string &fileUri) { return env->CallBooleanMethod(nativeActivity, contentUriRemoveFile, paramFileName); } -bool Android_GetFileInfo(const std::string &fileUri, FileInfo *info) { +bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *info) { if (!nativeActivity) { return -1; } @@ -338,7 +346,7 @@ class ContentURIFileLoader : public ProxiedFileLoader { public: ContentURIFileLoader(const Path &filename) : ProxiedFileLoader(nullptr) { // we overwrite the nullptr below - int fd = Android_OpenContentUriFd(filename.ToString()); + int fd = Android_OpenContentUriFd(filename.ToString(), Android_OpenContentUriMode::READ); INFO_LOG(SYSTEM, "Fd %d for content URI: '%s'", fd, filename.c_str()); backend_ = new LocalFileLoader(fd, filename); } @@ -607,7 +615,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeActivity_registerCallbacks(JNIEnv * postCommand = env->GetMethodID(env->GetObjectClass(obj), "postCommand", "(Ljava/lang/String;Ljava/lang/String;)V"); _dbg_assert_(postCommand); - openContentUri = env->GetMethodID(env->GetObjectClass(obj), "openContentUri", "(Ljava/lang/String;)I"); + openContentUri = env->GetMethodID(env->GetObjectClass(obj), "openContentUri", "(Ljava/lang/String;Ljava/lang/String;)I"); _dbg_assert_(openContentUri); listContentUriDir = env->GetMethodID(env->GetObjectClass(obj), "listContentUriDir", "(Ljava/lang/String;)[Ljava/lang/String;"); _dbg_assert_(listContentUriDir); diff --git a/android/jni/app-android.h b/android/jni/app-android.h index d24cc6e011..fec7c229e3 100644 --- a/android/jni/app-android.h +++ b/android/jni/app-android.h @@ -25,11 +25,20 @@ extern std::string g_extFilesDir; // Called from PathBrowser for example. bool Android_IsContentUri(const std::string &uri); -int Android_OpenContentUriFd(const std::string &uri); + +// To emphasize that Android storage mode strings are different, let's just use +// an enum. +enum class Android_OpenContentUriMode { + READ = 0, // "r" + READ_WRITE = 1, // "rw" + READ_WRITE_TRUNCATE = 2, // "rwt" +}; + +int Android_OpenContentUriFd(const std::string &uri, const Android_OpenContentUriMode mode); bool Android_CreateDirectory(const std::string &parentTreeUri, const std::string &dirName); bool Android_CreateFile(const std::string &parentTreeUri, const std::string &fileName); bool Android_RemoveFile(const std::string &fileUri); -bool Android_GetFileInfo(const std::string &fileUri, FileInfo *info); +bool Android_GetFileInfo(const std::string &fileUri, File::FileInfo *info); std::vector Android_ListContentUri(const std::string &uri); diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index e7e95abcca..190eb9132e 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -116,10 +116,10 @@ public class PpssppActivity extends NativeActivity { }); } - public int openContentUri(String uriString) { + public int openContentUri(String uriString, String mode) { try { Uri uri = Uri.parse(uriString); - ParcelFileDescriptor filePfd = getContentResolver().openFileDescriptor(uri, "r"); + ParcelFileDescriptor filePfd = getContentResolver().openFileDescriptor(uri, mode); if (filePfd == null) { Log.e(TAG, "Failed to get file descriptor for " + uriString); return -1;