diff --git a/Common/File/FileUtil.cpp b/Common/File/FileUtil.cpp index 218669cd68..46b92dac24 100644 --- a/Common/File/FileUtil.cpp +++ b/Common/File/FileUtil.cpp @@ -1080,6 +1080,41 @@ bool IsDirectoryWritable(const Path &path) { return true; } +bool SetFileWritable(const Path &filename, bool writable) { + switch (filename.Type()) { + case PathType::NATIVE: + break; + default: + // Content URIs and the other virtual path types have no notion of this, so say so rather + // than pretending it worked. + return false; + } + +#ifdef _WIN32 + const DWORD attrs = GetFileAttributesW(filename.ToWString().c_str()); + if (attrs == INVALID_FILE_ATTRIBUTES) { + return false; + } + const DWORD updated = writable ? (attrs & ~FILE_ATTRIBUTE_READONLY) : (attrs | FILE_ATTRIBUTE_READONLY); + if (updated == attrs) { + return true; + } + return SetFileAttributesW(filename.ToWString().c_str(), updated) != 0; +#else + struct stat info; + if (stat(filename.c_str(), &info) != 0) { + return false; + } + // Mirror the write bits onto whoever already has read access, which is what clearing the FAT + // read-only attribute amounts to. + mode_t mode = info.st_mode & ~(mode_t)0222; + if (writable) { + mode |= (info.st_mode & 0444) >> 1; + } + return chmod(filename.c_str(), mode) == 0; +#endif +} + // Deletes an empty directory, returns true on success // WARNING: On Android with content URIs, it will delete recursively! bool DeleteDir(const Path &path) { diff --git a/Common/File/FileUtil.h b/Common/File/FileUtil.h index 9b5a0e12cf..23573c1911 100644 --- a/Common/File/FileUtil.h +++ b/Common/File/FileUtil.h @@ -127,6 +127,10 @@ bool CreateEmptyFile(const Path &filename); // don't tell the whole story (Windows ACLs, read-only mounts, ...). bool IsDirectoryWritable(const Path &path); +// Set or clear a file's read-only-ness, which is what a FAT read-only attribute maps onto. +// Returns false where the platform can't express it - notably Android content URIs. +bool SetFileWritable(const Path &filename, bool writable); + // Opens ini file (cheats, texture replacements etc.) // TODO: Belongs in System or something. bool OpenFileInEditor(const Path &fileName); diff --git a/Core/FileSystems/DirectoryFileSystem.cpp b/Core/FileSystems/DirectoryFileSystem.cpp index 6bc1fa57e9..2a8a0c2f50 100644 --- a/Core/FileSystems/DirectoryFileSystem.cpp +++ b/Core/FileSystems/DirectoryFileSystem.cpp @@ -634,6 +634,11 @@ bool DirectoryFileSystem::RemoveFile(const std::string &filename) { return ReplayApplyDisk(ReplayAction::FILE_REMOVE, retValue, CoreTiming::GetGlobalTimeUs()) != 0; } +bool DirectoryFileSystem::SetFileWritable(const std::string &filename, bool writable) { + Path fullName = GetLocalPath(filename); + return File::SetFileWritable(fullName, writable); +} + // Note that this runs *before* the literal path is tried, not as a fallback after it fails. That's // deliberate: on Windows the host resolves its own 8.3 aliases, which are generated by a different // rule than ours, so opening the literal name can quietly land on a different file than the one we diff --git a/Core/FileSystems/DirectoryFileSystem.h b/Core/FileSystems/DirectoryFileSystem.h index 7d0a276397..70d0411d1c 100644 --- a/Core/FileSystems/DirectoryFileSystem.h +++ b/Core/FileSystems/DirectoryFileSystem.h @@ -85,6 +85,7 @@ 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 SetFileWritable(const std::string &filename, bool writable) override; FileSystemFlags Flags() const override { return flags; } u64 FreeDiskSpace(const std::string &path) override; diff --git a/Core/FileSystems/FileSystem.cpp b/Core/FileSystems/FileSystem.cpp index c45ac269a1..cf24c002a5 100644 --- a/Core/FileSystems/FileSystem.cpp +++ b/Core/FileSystems/FileSystem.cpp @@ -103,6 +103,30 @@ static std::string CleanShortNamePart(std::string_view part, size_t maxLen, bool return out; } +// Whether the name's capitalisation survives without a long-name entry. FAT keeps one flag for +// the base and one for the extension, but the PSP only honours the base one - so a lowercase +// extension forces a long name entry, and with it a ~1 suffix, while a lowercase base alone +// doesn't. That's why hardware gives "shrt" -> SHRT but "readme.txt" -> README~1.TXT. +static bool ShortNameCaseSurvives(std::string_view base, std::string_view ext) { + bool lower = false, upper = false; + for (char c : base) { + if (c >= 'a' && c <= 'z') { + lower = true; + } else if (c >= 'A' && c <= 'Z') { + upper = true; + } + } + if (lower && upper) { + return false; + } + for (char c : ext) { + if (c >= 'a' && c <= 'z') { + return false; + } + } + return true; +} + void GenerateFatShortNames(const std::vector &listing, std::vector *shortNames) { shortNames->clear(); shortNames->reserve(listing.size()); @@ -128,6 +152,10 @@ void GenerateFatShortNames(const std::vector &listing, std::vector< lossy = true; } + if (!ShortNameCaseSurvives(baseIn, extIn)) { + lossy = true; + } + std::string base = CleanShortNamePart(baseIn, 8, &lossy); std::string ext = CleanShortNamePart(extIn, 3, &lossy); if (base.empty()) { diff --git a/Core/FileSystems/FileSystem.h b/Core/FileSystems/FileSystem.h index 4e59d3811a..2dc25f9afb 100644 --- a/Core/FileSystems/FileSystem.h +++ b/Core/FileSystems/FileSystem.h @@ -157,6 +157,9 @@ public: virtual bool RmDir(const std::string &dirname) = 0; virtual int RenameFile(const std::string &from, const std::string &to) = 0; virtual bool RemoveFile(const std::string &filename) = 0; + // Sets or clears the FAT read-only attribute, as sceIoChstat does. Defaults to "can't", which + // is right for read-only filesystems and for hosts that can't express it. + virtual bool SetFileWritable(const std::string &filename, bool writable) { return false; } virtual int Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outdataPtr, u32 outlen, int &usec) = 0; virtual PSPDevType DevType(u32 handle) = 0; virtual FileSystemFlags Flags() const = 0; diff --git a/Core/FileSystems/MetaFileSystem.cpp b/Core/FileSystems/MetaFileSystem.cpp index f87cddba9a..d1065b17c3 100644 --- a/Core/FileSystems/MetaFileSystem.cpp +++ b/Core/FileSystems/MetaFileSystem.cpp @@ -507,6 +507,19 @@ int MetaFileSystem::RenameFile(const std::string &from, const std::string &to) } } +bool MetaFileSystem::SetFileWritable(const std::string &filename, bool writable) +{ + std::lock_guard guard(lock); + std::string of; + IFileSystem *system; + int error = MapFilePath(filename, &of, &system); + if (error == 0) { + return system->SetFileWritable(of, writable); + } else { + return false; + } +} + bool MetaFileSystem::RemoveFile(const std::string &filename) { std::lock_guard guard(lock); diff --git a/Core/FileSystems/MetaFileSystem.h b/Core/FileSystems/MetaFileSystem.h index dc71263b99..ffbde32dd1 100644 --- a/Core/FileSystems/MetaFileSystem.h +++ b/Core/FileSystems/MetaFileSystem.h @@ -137,6 +137,7 @@ 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 SetFileWritable(const std::string &filename, bool writable) override; int Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outdataPtr, u32 outlen, int &usec) override; PSPDevType DevType(u32 handle) override; FileSystemFlags Flags() const override { return FileSystemFlags::NONE; } diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index 5b3d33fb5e..2418c5bca2 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -871,9 +871,12 @@ void ConvertTmToPspDateTime(ScePspDateTime& date_out, const tm& date_in, int mic date_out.microsecond = microSeconds; } -static void __IoGetStat(SceIoStat *stat, const PSPFileInfo &info) { - memset(stat, 0xfe, sizeof(SceIoStat)); - +// isFAT is whether the file lives on a FAT volume (the memory stick), which changes both the +// permissions reported and whether st_private means anything. +static void __IoGetStat(SceIoStat *stat, const PSPFileInfo &info, bool isFAT) { + // Deliberately no memset: pspautotests io/stat poisons the struct and shows a real PSP writes + // only as far as the timestamps - the six st_private words come back exactly as the caller + // left them. Clearing the whole struct would destroy 24 bytes the kernel never touches. int type, attr; if (info.type & FILETYPE_DIRECTORY) { type = SCE_STM_FDIR; @@ -883,13 +886,26 @@ static void __IoGetStat(SceIoStat *stat, const PSPFileInfo &info) { attr = TYPE_FILE; } - stat->st_mode = type | info.access; - stat->st_attr = attr; + if (isFAT) { + // FAT has no permissions of its own, so everything reads back as 0777 - including the + // execute bits, which is what Beats needed (issue #14812). Clearing the write bits is + // the read-only attribute, and that shows up in st_attr too. + const bool readOnly = (info.access & 0222) == 0; + stat->st_mode = type | (readOnly ? 0555 : 0777); + stat->st_attr = attr | (readOnly ? 0x01 : 0x00); + } else { + stat->st_mode = type | info.access; + stat->st_attr = attr; + } stat->st_size = info.size; ConvertTmToPspDateTime(stat->st_a_time, info.atime, info.atimeUs); ConvertTmToPspDateTime(stat->st_c_time, info.ctime, info.ctimeUs); ConvertTmToPspDateTime(stat->st_m_time, info.mtime, info.mtimeUs); - stat->st_private[0] = info.startSector; + // st_private[0] carries the LBN on a UMD, which games read to build disc0:/sce_lbn paths - + // see umd/raw_access. On the memory stick a real PSP leaves it alone entirely. + if (!isFAT) { + stat->st_private[0] = info.startSector; + } } static void __IoSchedAsync(FileNode *f, int fd, int usec) { @@ -911,11 +927,25 @@ static u32 sceIoGetstat(const char *filename, u32 addr) { // TODO: Improve timing (although this seems normally slow..) int usec = 1000; + // A real PSP refuses to stat the root of a volume - io/stat records sceIoGetstat("ms0:/") + // coming back as an invalid argument rather than describing the directory. + const char *colon = strchr(filename, ':'); + if (colon != nullptr) { + const char *rest = colon + 1; + while (*rest == '/') { + ++rest; + } + if (*rest == '\0') { + return hleDelayResult(hleLogWarning(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "volume root"), "io getstat", usec); + } + } + + const bool isFAT = pspFileSystem.FlagsFromFilename(filename) & FileSystemFlags::SIMULATE_FAT32; auto stat = PSPPointer::Create(addr); PSPFileInfo info = pspFileSystem.GetFileInfo(filename); if (info.exists) { if (stat.IsValid()) { - __IoGetStat(stat, info); + __IoGetStat(stat, info, isFAT); stat.NotifyWrite("IoGetstat"); return hleDelayResult(hleLogDebug(Log::sceIo, 0, "sector = %08x", info.startSector), "io getstat", usec); } else { @@ -931,12 +961,26 @@ static u32 sceIoChstat(const char *filename, u32 iostatptr, u32 changebits) { if (!iostat.IsValid()) return hleReportError(Log::sceIo, SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT, "bad address"); - ERROR_LOG(Log::sceIo, "UNIMPL sceIoChstat(%s, %08x, %08x)", filename, iostatptr, changebits); - if (changebits & SCE_CST_MODE) - ERROR_LOG_REPORT(Log::sceIo, "sceIoChstat: change mode to %03o requested", iostat->st_mode); + // On a FAT volume the write bits in st_mode and the 0x01 bit in st_attr are two views of the + // same read-only flag: io/stat/readonly records that setting either one produces both, and + // that it's reversible. Anything else in the struct is still ignored. + bool haveWritable = false; + bool writable = false; + if (changebits & SCE_CST_MODE) { + writable = (iostat->st_mode & 0222) != 0; + haveWritable = true; + } if (changebits & SCE_CST_ATTR) { - // These are pretty much all of the reported calls: https://report.ppsspp.org/logs/kind/1115 - ERROR_LOG_REPORT(Log::sceIo, "sceIoChstat: change attr to %04x requested", iostat->st_attr); + // The attribute wins if both were asked for, since it names the flag directly. + writable = (iostat->st_attr & 0x01) == 0; + haveWritable = true; + } + if (haveWritable) { + if (!pspFileSystem.SetFileWritable(filename, writable)) { + // Nothing to be done on a host that can't express it - Android content URIs, or a + // read-only filesystem. Hardware would have succeeded, so don't fail the call. + WARN_LOG(Log::sceIo, "sceIoChstat: could not make %s %s", filename, writable ? "writable" : "read-only"); + } } if (changebits & SCE_CST_SIZE) ERROR_LOG(Log::sceIo, "sceIoChstat: change size requested"); @@ -2533,17 +2577,15 @@ static u32 sceIoDread(int id, u32 dirent_addr) { } PSPFileInfo &info = dir->listing[dir->index]; - __IoGetStat(&entry->d_stat, info); + const bool isFATDir = pspFileSystem.FlagsFromFilename(dir->name) & FileSystemFlags::SIMULATE_FAT32; + __IoGetStat(&entry->d_stat, info, isFATDir); strncpy(entry->d_name, info.name.c_str(), 256); entry->d_name[255] = '\0'; - bool isFAT = pspFileSystem.FlagsFromFilename(dir->name) & FileSystemFlags::SIMULATE_FAT32; // Only write d_private for memory stick - if (isFAT) { + if (isFATDir) { const std::string &shortName = dir->ShortName(dir->index); - // All files look like they're executable on FAT. This is required for Beats, see issue #14812 - entry->d_stat.st_mode |= 0111; // write d_private for supporting Custom BGM // ref JPCSP https://code.google.com/p/jpcsp/source/detail?r=3468 if (Memory::IsValidAddress(entry->d_private)){ diff --git a/pspautotests b/pspautotests index dcc31bc21c..f25996c927 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit dcc31bc21cab554c8cff226898a93c84e7b90ae3 +Subproject commit f25996c927efacd225b590fee4d53a5efeceee4b diff --git a/test.py b/test.py index e66c040a90..22142cb689 100755 --- a/test.py +++ b/test.py @@ -221,6 +221,8 @@ tests_good = [ "io/cwd/cwd", "io/file/rename", "io/directory/directory", + "io/stat/stat", + "io/stat/readonly", "io/open/badparent", "jpeg/create", "jpeg/delete", @@ -466,7 +468,6 @@ tests_next = [ "io/io/io", "io/iodrv/iodrv", "io/shortname/shortname", - "io/stat/stat", "io/open/tty0", "jpeg/csc", "jpeg/decode", diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 9086c598f1..fc4657c9f7 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -2914,14 +2914,29 @@ bool TestFatShortNames() { return shortNames; }; - // Names that already fit 8.3 are only uppercased, and the navigation entries are left alone. + // A name that is already valid uppercase 8.3 is kept as-is, and the navigation entries are + // left alone. "readme.md" is not: its extension is lowercase, which a PSP can't record, so it + // gets a counter - see the case block below. std::vector plain = shortNamesFor({".", "..", "TEST.TXT", "readme.md", "WIPEOUT"}); EXPECT_EQ_STR(plain[0], std::string(".")); EXPECT_EQ_STR(plain[1], std::string("..")); EXPECT_EQ_STR(plain[2], std::string("TEST.TXT")); - EXPECT_EQ_STR(plain[3], std::string("README.MD")); + EXPECT_EQ_STR(plain[3], std::string("README~1.MD")); EXPECT_EQ_STR(plain[4], std::string("WIPEOUT")); + // Capitalisation, as recorded off a real PSP by pspautotests io/shortname. FAT keeps a + // lowercase flag for the base and another for the extension, but the PSP only honours the + // base one - so a lowercase base survives on its own and a lowercase extension never does. + std::vector cased = shortNamesFor({"shrt", "readme.txt", "UPPER.TXT", "MiXeD.txt"}); + // All lowercase, no extension: representable, so no counter. + EXPECT_EQ_STR(cased[0], std::string("SHRT")); + // Lowercase extension: not representable. + EXPECT_EQ_STR(cased[1], std::string("README~1.TXT")); + // Already uppercase throughout. + EXPECT_EQ_STR(cased[2], std::string("UPPER.TXT")); + // Mixed case in the base. + EXPECT_EQ_STR(cased[3], std::string("MIXED~1.TXT")); + // Long names get truncated to six characters plus a counter, which keeps counting past ~4. std::vector many = shortNamesFor({ "sample-12s.mp3", @@ -2945,9 +2960,23 @@ bool TestFatShortNames() { std::vector odd = shortNamesFor({"my song.mp3", "a+b.mp3", "no_ext", ".hidden"}); EXPECT_EQ_STR(odd[0], std::string("MYSONG~1.MP3")); EXPECT_EQ_STR(odd[1], std::string("A_B~1.MP3")); + // All lowercase with no extension, so this one keeps its name. EXPECT_EQ_STR(odd[2], std::string("NO_EXT")); EXPECT_EQ_STR(odd[3], std::string("HIDDEN~1")); + // The rest of what io/shortname records, so the whole recorded set is pinned here and not + // only in a test that needs a PSP to re-run. + std::vector hw = shortNamesFor({ + "a.b.c.txt", "noextensionhere", "sp ace.txt", "+plus[brack].txt", + "toolongextension.mpeg", "LongDirectoryName", + }); + EXPECT_EQ_STR(hw[0], std::string("ABC~1.TXT")); + EXPECT_EQ_STR(hw[1], std::string("NOEXTE~1")); + EXPECT_EQ_STR(hw[2], std::string("SPACE~1.TXT")); + EXPECT_EQ_STR(hw[3], std::string("_PLUS_~1.TXT")); + EXPECT_EQ_STR(hw[4], std::string("TOOLON~1.MPE")); + EXPECT_EQ_STR(hw[5], std::string("LONGDI~1")); + // Two long names sharing a six character stem must not collide. std::vector collide = shortNamesFor({"longname-one.txt", "longname-two.txt"}); EXPECT_EQ_STR(collide[0], std::string("LONGNA~1.TXT"));