From 2d8be6a77d4afe8e863119c85c2c2cc472c0540b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Thu, 8 Mar 2018 18:32:49 +0100 Subject: [PATCH] IOS/FS: Make it harder to forget closing FDs Return a FileHandle which will automatically close the FD when the handle goes out of scope. For the rare cases where this behaviour is undesirable, the FD can be released from the handle. --- Source/Core/Core/Boot/Boot_WiiWAD.cpp | 1 - Source/Core/Core/IOS/FS/FileSystem.cpp | 30 ++++++++++++++++++++ Source/Core/Core/IOS/FS/FileSystem.h | 25 ++++++++++++++-- Source/Core/Core/IOS/FS/FileSystemProxy.cpp | 6 ++-- Source/Core/Core/IOS/FS/HostBackend/FS.h | 2 +- Source/Core/Core/IOS/FS/HostBackend/File.cpp | 4 +-- 6 files changed, 59 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/Boot/Boot_WiiWAD.cpp b/Source/Core/Core/Boot/Boot_WiiWAD.cpp index 5ecfd48471..111f87de3b 100644 --- a/Source/Core/Core/Boot/Boot_WiiWAD.cpp +++ b/Source/Core/Core/Boot/Boot_WiiWAD.cpp @@ -49,7 +49,6 @@ static void CreateVirtualFATFilesystem(std::shared_ptr // write the final 0 to 0 file from the second FAT to 20 MiB data[CDB_SIZE - 1] = 0; fs->WriteFile(*fd, data.data(), static_cast(data.size())); - fs->Close(*fd); } bool CBoot::BootNANDTitle(const u64 title_id) diff --git a/Source/Core/Core/IOS/FS/FileSystem.cpp b/Source/Core/Core/IOS/FS/FileSystem.cpp index 50aa764ee7..f3910c3374 100644 --- a/Source/Core/Core/IOS/FS/FileSystem.cpp +++ b/Source/Core/Core/IOS/FS/FileSystem.cpp @@ -4,6 +4,7 @@ #include "Core/IOS/FS/FileSystem.h" +#include "Common/Assert.h" #include "Core/IOS/FS/HostBackend/FS.h" namespace IOS::HLE::FS @@ -13,6 +14,35 @@ std::unique_ptr MakeFileSystem() return std::make_unique(); } +FileHandle::FileHandle(FileSystem* fs, Fd fd) : m_fs{fs}, m_fd{fd} +{ +} + +FileHandle::FileHandle(FileHandle&& other) : m_fs{other.m_fs}, m_fd{other.m_fd} +{ + other.m_fd.reset(); +} + +FileHandle& FileHandle::operator=(FileHandle&& other) +{ + if (*this != other) + *this = std::move(other); + return *this; +} + +FileHandle::~FileHandle() +{ + if (m_fd && m_fs) + ASSERT(m_fs->Close(*m_fd) == FS::ResultCode::Success); +} + +Fd FileHandle::Release() +{ + const Fd fd = m_fd.value(); + m_fd.reset(); + return fd; +} + void FileSystem::Init() { if (Delete(0, 0, "/tmp") == ResultCode::Success) diff --git a/Source/Core/Core/IOS/FS/FileSystem.h b/Source/Core/Core/IOS/FS/FileSystem.h index 94fc430230..25894a60b0 100644 --- a/Source/Core/Core/IOS/FS/FileSystem.h +++ b/Source/Core/Core/IOS/FS/FileSystem.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include @@ -96,6 +97,26 @@ struct FileStatus u32 size; }; +class FileSystem; +class FileHandle final +{ +public: + FileHandle(FileSystem* fs, Fd fd); + FileHandle(FileHandle&&); + ~FileHandle(); + FileHandle(const FileHandle&) = delete; + FileHandle& operator=(const FileHandle&) = delete; + FileHandle& operator=(FileHandle&&); + + operator Fd() const { return m_fd.value(); } + /// Release the FD so that it is not automatically closed. + Fd Release(); + +private: + FileSystem* m_fs; + std::optional m_fd; +}; + class FileSystem { public: @@ -106,8 +127,8 @@ public: /// Format the file system. virtual ResultCode Format(Uid uid) = 0; - /// Get a file descriptor for accessing a file. - virtual Result OpenFile(Uid uid, Gid gid, const std::string& path, Mode mode) = 0; + /// Get a file descriptor for accessing a file. The FD will be automatically closed after use. + virtual Result OpenFile(Uid uid, Gid gid, const std::string& path, Mode mode) = 0; /// Close a file descriptor. virtual ResultCode Close(Fd fd) = 0; /// Read `size` bytes from the file descriptor. Returns the number of bytes read. diff --git a/Source/Core/Core/IOS/FS/FileSystemProxy.cpp b/Source/Core/Core/IOS/FS/FileSystemProxy.cpp index e73d502f8a..75d0c6c6b4 100644 --- a/Source/Core/Core/IOS/FS/FileSystemProxy.cpp +++ b/Source/Core/Core/IOS/FS/FileSystemProxy.cpp @@ -70,13 +70,13 @@ IPCCommandResult FS::Open(const OpenRequest& request) return GetDefaultReply(IPC_SUCCESS); } - const auto backend_fd = m_ios.GetFS()->OpenFile(request.uid, request.gid, request.path, - static_cast(request.flags & 3)); + auto backend_fd = m_ios.GetFS()->OpenFile(request.uid, request.gid, request.path, + static_cast(request.flags & 3)); LogResult(StringFromFormat("OpenFile(%s)", request.path.c_str()), backend_fd); if (!backend_fd) return GetFSReply(ConvertResult(backend_fd.Error())); - m_fd_map[request.fd] = {request.gid, request.uid, *backend_fd}; + m_fd_map[request.fd] = {request.gid, request.uid, backend_fd->Release()}; std::strncpy(m_fd_map[request.fd].name.data(), request.path.c_str(), 64); return GetFSReply(IPC_SUCCESS); } diff --git a/Source/Core/Core/IOS/FS/HostBackend/FS.h b/Source/Core/Core/IOS/FS/HostBackend/FS.h index cb7283d1f0..60f5d4dfd4 100644 --- a/Source/Core/Core/IOS/FS/HostBackend/FS.h +++ b/Source/Core/Core/IOS/FS/HostBackend/FS.h @@ -30,7 +30,7 @@ public: ResultCode Format(Uid uid) override; - Result OpenFile(Uid uid, Gid gid, const std::string& path, Mode mode) override; + Result OpenFile(Uid uid, Gid gid, const std::string& path, Mode mode) override; ResultCode Close(Fd fd) override; Result ReadBytesFromFile(Fd fd, u8* ptr, u32 size) override; Result WriteBytesToFile(Fd fd, const u8* ptr, u32 size) override; diff --git a/Source/Core/Core/IOS/FS/HostBackend/File.cpp b/Source/Core/Core/IOS/FS/HostBackend/File.cpp index 405c877c45..661413300d 100644 --- a/Source/Core/Core/IOS/FS/HostBackend/File.cpp +++ b/Source/Core/Core/IOS/FS/HostBackend/File.cpp @@ -57,7 +57,7 @@ std::shared_ptr HostFileSystem::OpenHostFile(const std::string& ho return file; } -Result HostFileSystem::OpenFile(Uid uid, Gid gid, const std::string& path, Mode mode) +Result HostFileSystem::OpenFile(Uid, Gid, const std::string& path, Mode mode) { Handle* handle = AssignFreeHandle(); if (!handle) @@ -74,7 +74,7 @@ Result HostFileSystem::OpenFile(Uid uid, Gid gid, const std::string& path, M handle->wii_path = path; handle->mode = mode; handle->file_offset = 0; - return ConvertHandleToFd(handle); + return FileHandle{this, ConvertHandleToFd(handle)}; } ResultCode HostFileSystem::Close(Fd fd)