Merge pull request #192 from bunnei/fs-fix-paths

FileSys: Updates backend code to use FileSys::Path and fixes binary path types.

src/core/file_sys/archive.h

@@ -74,6 +74,35 @@ public:
         return type;
     }
 
+    /**
+     * Gets the string representation of the path for debugging
+     * @return String representation of the path for debugging
+     */
+    const std::string DebugStr() const {
+        switch (GetType()) {
+        case Invalid:
+            return "[Invalid]";
+        case Empty:
+            return "[Empty]";
+        case Binary:
+        {
+            std::stringstream res;
+            res << "[Binary: ";
+            for (unsigned byte : binary)
+                res << std::hex << std::setw(2) << std::setfill('0') << byte;
+            res << ']';
+            return res.str();
+        }
+        case Char:
+            return "[Char: " + AsString() + ']';
+        case Wchar:
+            return "[Wchar: " + AsString() + ']';
+        default:
+            ERROR_LOG(KERNEL, "LowPathType cannot be converted to string!");
+            return {};
+        }
+    }
+
     const std::string AsString() const {
         switch (GetType()) {
             case Char:
@@ -153,21 +182,21 @@ public:
      * @param mode Mode to open the file with
      * @return Opened file, or nullptr
      */
-    virtual std::unique_ptr<File> OpenFile(const std::string& path, const Mode mode) const = 0;
+    virtual std::unique_ptr<File> OpenFile(const Path& path, const Mode mode) const = 0;
 
     /**
      * Create a directory specified by its path
      * @param path Path relative to the archive
      * @return Whether the directory could be created
      */
-    virtual bool CreateDirectory(const std::string& path) const = 0;
+    virtual bool CreateDirectory(const Path& path) const = 0;
 
     /**
      * Open a directory specified by its path
      * @param path Path relative to the archive
      * @return Opened directory, or nullptr
      */
-    virtual std::unique_ptr<Directory> OpenDirectory(const std::string& path) const = 0;
+    virtual std::unique_ptr<Directory> OpenDirectory(const Path& path) const = 0;
 
     /**
      * Read data from the archive

src/core/file_sys/archive_romfs.cpp

@@ -29,7 +29,7 @@ Archive_RomFS::~Archive_RomFS() {
  * @param mode Mode to open the file with
  * @return Opened file, or nullptr
  */
-std::unique_ptr<File> Archive_RomFS::OpenFile(const std::string& path, const Mode mode) const {
+std::unique_ptr<File> Archive_RomFS::OpenFile(const Path& path, const Mode mode) const {
     return std::unique_ptr<File>(new File_RomFS);
 }
 
@@ -38,7 +38,7 @@ std::unique_ptr<File> Archive_RomFS::OpenFile(const std::string& path, const Mod
  * @param path Path relative to the archive
  * @return Whether the directory could be created
  */
-bool Archive_RomFS::CreateDirectory(const std::string& path) const {
+bool Archive_RomFS::CreateDirectory(const Path& path) const {
     ERROR_LOG(FILESYS, "Attempted to create a directory in ROMFS.");
     return false;
 };
@@ -48,7 +48,7 @@ bool Archive_RomFS::CreateDirectory(const std::string& path) const {
  * @param path Path relative to the archive
  * @return Opened directory, or nullptr
  */
-std::unique_ptr<Directory> Archive_RomFS::OpenDirectory(const std::string& path) const {
+std::unique_ptr<Directory> Archive_RomFS::OpenDirectory(const Path& path) const {
     return std::unique_ptr<Directory>(new Directory_RomFS);
 }
 

src/core/file_sys/archive_romfs.h

@@ -34,21 +34,21 @@ public:
      * @param mode Mode to open the file with
      * @return Opened file, or nullptr
      */
-    std::unique_ptr<File> OpenFile(const std::string& path, const Mode mode) const override;
+    std::unique_ptr<File> OpenFile(const Path& path, const Mode mode) const override;
 
     /**
      * Create a directory specified by its path
      * @param path Path relative to the archive
      * @return Whether the directory could be created
      */
-    bool CreateDirectory(const std::string& path) const override;
+    bool CreateDirectory(const Path& path) const override;
 
     /**
      * Open a directory specified by its path
      * @param path Path relative to the archive
      * @return Opened directory, or nullptr
      */
-    std::unique_ptr<Directory> OpenDirectory(const std::string& path) const override;
+    std::unique_ptr<Directory> OpenDirectory(const Path& path) const override;
 
     /**
      * Read data from the archive

src/core/file_sys/archive_sdmc.cpp

@@ -49,8 +49,8 @@ bool Archive_SDMC::Initialize() {
  * @param mode Mode to open the file with
  * @return Opened file, or nullptr
  */
-std::unique_ptr<File> Archive_SDMC::OpenFile(const std::string& path, const Mode mode) const {
+std::unique_ptr<File> Archive_SDMC::OpenFile(const Path& path, const Mode mode) const {
-    DEBUG_LOG(FILESYS, "called path=%s mode=%d", path.c_str(), mode);
+    DEBUG_LOG(FILESYS, "called path=%s mode=%d", path.DebugStr().c_str(), mode);
     File_SDMC* file = new File_SDMC(this, path, mode);
     if (!file->Open())
         return nullptr;
@@ -62,8 +62,8 @@ std::unique_ptr<File> Archive_SDMC::OpenFile(const std::string& path, const Mode
  * @param path Path relative to the archive
  * @return Whether the directory could be created
  */
-bool Archive_SDMC::CreateDirectory(const std::string& path) const {
+bool Archive_SDMC::CreateDirectory(const Path& path) const {
-    return FileUtil::CreateDir(GetMountPoint() + path);
+    return FileUtil::CreateDir(GetMountPoint() + path.AsString());
 }
 
 /**
@@ -71,8 +71,8 @@ bool Archive_SDMC::CreateDirectory(const std::string& path) const {
  * @param path Path relative to the archive
  * @return Opened directory, or nullptr
  */
-std::unique_ptr<Directory> Archive_SDMC::OpenDirectory(const std::string& path) const {
+std::unique_ptr<Directory> Archive_SDMC::OpenDirectory(const Path& path) const {
-    DEBUG_LOG(FILESYS, "called path=%s", path.c_str());
+    DEBUG_LOG(FILESYS, "called path=%s", path.DebugStr().c_str());
     Directory_SDMC* directory = new Directory_SDMC(this, path);
     return std::unique_ptr<Directory>(directory);
 }

src/core/file_sys/archive_sdmc.h

@@ -38,21 +38,21 @@ public:
      * @param mode Mode to open the file with
      * @return Opened file, or nullptr
      */
-    std::unique_ptr<File> OpenFile(const std::string& path, const Mode mode) const override;
+    std::unique_ptr<File> OpenFile(const Path& path, const Mode mode) const override;
 
     /**
      * Create a directory specified by its path
      * @param path Path relative to the archive
      * @return Whether the directory could be created
      */
-    bool CreateDirectory(const std::string& path) const override;
+    bool CreateDirectory(const Path& path) const override;
 
     /**
      * Open a directory specified by its path
      * @param path Path relative to the archive
      * @return Opened directory, or nullptr
      */
-    std::unique_ptr<Directory> OpenDirectory(const std::string& path) const override;
+    std::unique_ptr<Directory> OpenDirectory(const Path& path) const override;
 
     /**
      * Read data from the archive

src/core/file_sys/directory_sdmc.cpp

@@ -15,11 +15,11 @@
 
 namespace FileSys {
 
-Directory_SDMC::Directory_SDMC(const Archive_SDMC* archive, const std::string& path) {
+Directory_SDMC::Directory_SDMC(const Archive_SDMC* archive, const Path& path) {
     // TODO(Link Mauve): normalize path into an absolute path without "..", it can currently bypass
     // the root directory we set while opening the archive.
     // For example, opening /../../usr/bin can give the emulated program your installed programs.
-    std::string absolute_path = archive->GetMountPoint() + path;
+    std::string absolute_path = archive->GetMountPoint() + path.AsString();
     FileUtil::ScanDirectoryTree(absolute_path, directory);
     children_iterator = directory.children.begin();
 }

src/core/file_sys/directory_sdmc.h

@@ -19,7 +19,7 @@ namespace FileSys {
 class Directory_SDMC final : public Directory {
 public:
     Directory_SDMC();
-    Directory_SDMC(const Archive_SDMC* archive, const std::string& path);
+    Directory_SDMC(const Archive_SDMC* archive, const Path& path);
     ~Directory_SDMC() override;
 
     /**

src/core/file_sys/file_sdmc.cpp

@@ -15,11 +15,11 @@
 
 namespace FileSys {
 
-File_SDMC::File_SDMC(const Archive_SDMC* archive, const std::string& path, const Mode mode) {
+File_SDMC::File_SDMC(const Archive_SDMC* archive, const Path& path, const Mode mode) {
     // TODO(Link Mauve): normalize path into an absolute path without "..", it can currently bypass
     // the root directory we set while opening the archive.
     // For example, opening /../../etc/passwd can give the emulated program your users list.
-    this->path = archive->GetMountPoint() + path;
+    this->path = archive->GetMountPoint() + path.AsString();
     this->mode.hex = mode.hex;
 }
 

src/core/file_sys/file_sdmc.h

@@ -19,7 +19,7 @@ namespace FileSys {
 class File_SDMC final : public File {
 public:
     File_SDMC();
-    File_SDMC(const Archive_SDMC* archive, const std::string& path, const Mode mode);
+    File_SDMC(const Archive_SDMC* archive, const Path& path, const Mode mode);
     ~File_SDMC() override;
 
     /**

src/core/hle/kernel/archive.cpp

@@ -99,7 +99,6 @@ public:
         case FileCommand::Close:
         {
             DEBUG_LOG(KERNEL, "Close %s %s", GetTypeName().c_str(), GetName().c_str());
-            Kernel::g_object_pool.Destroy<Archive>(GetHandle());
             CloseArchive(backend->GetIdCode());
             break;
         }
@@ -129,12 +128,12 @@ public:
 class File : public Object {
 public:
     std::string GetTypeName() const override { return "File"; }
-    std::string GetName() const override { return path; }
+    std::string GetName() const override { return path.DebugStr(); }
 
     static Kernel::HandleType GetStaticHandleType() { return HandleType::File; }
     Kernel::HandleType GetHandleType() const override { return HandleType::File; }
 
-    std::string path; ///< Path of the file
+    FileSys::Path path; ///< Path of the file
     std::unique_ptr<FileSys::File> backend; ///< File backend interface
 
     /**
@@ -221,12 +220,12 @@ public:
 class Directory : public Object {
 public:
     std::string GetTypeName() const override { return "Directory"; }
-    std::string GetName() const override { return path; }
+    std::string GetName() const override { return path.DebugStr(); }
 
     static Kernel::HandleType GetStaticHandleType() { return HandleType::Directory; }
     Kernel::HandleType GetHandleType() const override { return HandleType::Directory; }
 
-    std::string path; ///< Path of the directory
+    FileSys::Path path; ///< Path of the directory
     std::unique_ptr<FileSys::Directory> backend; ///< File backend interface
 
     /**
@@ -304,8 +303,9 @@ Handle OpenArchive(FileSys::Archive::IdCode id_code) {
  * @return Result of operation, 0 on success, otherwise error code
  */
 Result CloseArchive(FileSys::Archive::IdCode id_code) {
-    if (1 != g_archive_map.erase(id_code)) {
+    auto itr = g_archive_map.find(id_code);
-        ERROR_LOG(KERNEL, "Cannot close archive %d", (int) id_code);
+    if (itr == g_archive_map.end()) {
+        ERROR_LOG(KERNEL, "Cannot close archive %d, does not exist!", (int)id_code);
         return -1;
     }
 
@@ -366,7 +366,18 @@ Handle CreateArchive(FileSys::Archive* backend, const std::string& name) {
  * @param mode Mode under which to open the File
  * @return Opened File object
  */
-Handle OpenFileFromArchive(Handle archive_handle, const std::string& path, const FileSys::Mode mode) {
+Handle OpenFileFromArchive(Handle archive_handle, const FileSys::Path& path, const FileSys::Mode mode) {
+    // TODO(bunnei): Binary type files get a raw file pointer to the archive. Currently, we create
+    // the archive file handles at app loading, and then keep them persistent throughout execution.
+    // Archives file handles are just reused and not actually freed until emulation shut down.
+    // Verify if real hardware works this way, or if new handles are created each time
+    if (path.GetType() == FileSys::Binary)
+        // TODO(bunnei): FixMe - this is a hack to compensate for an incorrect FileSys backend
+        // design. While the functionally of this is OK, our implementation decision to separate
+        // normal files from archive file pointers is very likely wrong.
+        // See https://github.com/citra-emu/citra/issues/205
+        return archive_handle;
+
     File* file = new File;
     Handle handle = Kernel::g_object_pool.Create(file);
 
@@ -386,7 +397,7 @@ Handle OpenFileFromArchive(Handle archive_handle, const std::string& path, const
  * @param path Path to the Directory inside of the Archive
  * @return Opened Directory object
  */
-Result CreateDirectoryFromArchive(Handle archive_handle, const std::string& path) {
+Result CreateDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path) {
     Archive* archive = Kernel::g_object_pool.GetFast<Archive>(archive_handle);
     if (archive == nullptr)
         return -1;
@@ -401,7 +412,7 @@ Result CreateDirectoryFromArchive(Handle archive_handle, const std::string& path
  * @param path Path to the Directory inside of the Archive
  * @return Opened Directory object
  */
-Handle OpenDirectoryFromArchive(Handle archive_handle, const std::string& path) {
+Handle OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path) {
     Directory* directory = new Directory;
     Handle handle = Kernel::g_object_pool.Create(directory);
 

src/core/hle/kernel/archive.h

@@ -43,7 +43,7 @@ Handle CreateArchive(FileSys::Archive* backend, const std::string& name);
  * @param mode Mode under which to open the File
  * @return Opened File object
  */
-Handle OpenFileFromArchive(Handle archive_handle, const std::string& name, const FileSys::Mode mode);
+Handle OpenFileFromArchive(Handle archive_handle, const FileSys::Path& path, const FileSys::Mode mode);
 
 /**
  * Create a Directory from an Archive
@@ -51,7 +51,7 @@ Handle OpenFileFromArchive(Handle archive_handle, const std::string& name, const
  * @param path Path to the Directory inside of the Archive
  * @return Whether creation of directory succeeded
  */
-Result CreateDirectoryFromArchive(Handle archive_handle, const std::string& name);
+Result CreateDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path);
 
 /**
  * Open a Directory from an Archive
@@ -59,7 +59,7 @@ Result CreateDirectoryFromArchive(Handle archive_handle, const std::string& name
  * @param path Path to the Directory inside of the Archive
  * @return Opened Directory object
  */
-Handle OpenDirectoryFromArchive(Handle archive_handle, const std::string& name);
+Handle OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path);
 
 /// Initialize archives
 void ArchiveInit();

src/core/hle/service/fs_user.cpp

@@ -28,6 +28,22 @@ void Initialize(Service::Interface* self) {
     DEBUG_LOG(KERNEL, "called");
 }
 
+/**
+ * FS_User::OpenFile service function
+ *  Inputs:
+ *      1 : Transaction
+ *      2 : Archive handle lower word
+ *      3 : Archive handle upper word
+ *      4 : Low path type
+ *      5 : Low path size
+ *      6 : Open flags
+ *      7 : Attributes
+ *      8 : (LowPathSize << 14) | 2
+ *      9 : Low path data pointer
+ *  Outputs:
+ *      1 : Result of function, 0 on success, otherwise error code
+ *      3 : File handle
+ */
 void OpenFile(Service::Interface* self) {
     u32* cmd_buff = Service::GetCommandBuffer();
 
@@ -39,28 +55,16 @@ void OpenFile(Service::Interface* self) {
     FileSys::Mode mode; mode.hex = cmd_buff[6];
     u32 attributes        = cmd_buff[7]; // TODO(Link Mauve): do something with those attributes.
     u32 filename_ptr      = cmd_buff[9];
-
     FileSys::Path file_path(filename_type, filename_size, filename_ptr);
-    std::string file_string;
-    switch (file_path.GetType()) {
-    case FileSys::Char:
-    case FileSys::Wchar:
-        file_string = file_path.AsString();
-        break;
-    default:
-        WARN_LOG(KERNEL, "file LowPath type is currently unsupported; returning archive handle instead");
-        return;
-    }
 
-    DEBUG_LOG(KERNEL, "type=%d size=%d mode=%d attrs=%d data=%s",
+    DEBUG_LOG(KERNEL, "path=%s, mode=%d attrs=%d", file_path.DebugStr().c_str(), mode, attributes);
-              filename_type, filename_size, mode, attributes, file_string.c_str());
 
-    Handle handle = Kernel::OpenFileFromArchive(archive_handle, file_string, mode);
+    Handle handle = Kernel::OpenFileFromArchive(archive_handle, file_path, mode);
     if (handle) {
         cmd_buff[1] = 0;
         cmd_buff[3] = handle;
     } else {
-        ERROR_LOG(KERNEL, "failed to get a handle for file %s", file_string.c_str());
+        ERROR_LOG(KERNEL, "failed to get a handle for file %s", file_path.DebugStr().c_str());
         // TODO(Link Mauve): check for the actual error values, this one was just chosen arbitrarily.
         cmd_buff[1] = -1;
     }
@@ -68,6 +72,25 @@ void OpenFile(Service::Interface* self) {
     DEBUG_LOG(KERNEL, "called");
 }
 
+/**
+ * FS_User::OpenFileDirectly service function
+ *  Inputs:
+ *      1 : Transaction
+ *      2 : Archive ID
+ *      3 : Archive low path type
+ *      4 : Archive low path size
+ *      5 : File low path type
+ *      6 : File low path size
+ *      7 : Flags
+ *      8 : Attributes
+ *      9 : (ArchiveLowPathSize << 14) | 0x802
+ *      10 : Archive low path
+ *      11 : (FileLowPathSize << 14) | 2
+ *      12 : File low path
+ *  Outputs:
+ *      1 : Result of function, 0 on success, otherwise error code
+ *      3 : File handle
+ */
 void OpenFileDirectly(Service::Interface* self) {
     u32* cmd_buff = Service::GetCommandBuffer();
 
@@ -80,47 +103,33 @@ void OpenFileDirectly(Service::Interface* self) {
     u32 attributes        = cmd_buff[8]; // TODO(Link Mauve): do something with those attributes.
     u32 archivename_ptr   = cmd_buff[10];
     u32 filename_ptr      = cmd_buff[12];
+    FileSys::Path archive_path(archivename_type, archivename_size, archivename_ptr);
+    FileSys::Path file_path(filename_type, filename_size, filename_ptr);
 
-    DEBUG_LOG(KERNEL, "archive_type=%d archive_size=%d file_type=%d file_size=%d file_mode=%d file_attrs=%d",
+    DEBUG_LOG(KERNEL, "archive_path=%s file_path=%s, mode=%d attributes=%d",
-              archivename_type, archivename_size, filename_type, filename_size, mode, attributes);
+              archive_path.DebugStr().c_str(), file_path.DebugStr().c_str(), mode, attributes);
 
-    if (archivename_type != FileSys::Empty) {
+    if (archive_path.GetType() != FileSys::Empty) {
         ERROR_LOG(KERNEL, "archive LowPath type other than empty is currently unsupported");
         cmd_buff[1] = -1;
         return;
     }
 
-    // TODO(Link Mauve): check if we should even get a handle for the archive, and don't leak it.
+    // TODO(Link Mauve): Check if we should even get a handle for the archive, and don't leak it
     Handle archive_handle = Kernel::OpenArchive(archive_id);
-    if (archive_handle) {
+    if (!archive_handle) {
-        cmd_buff[1] = 0;
-        // cmd_buff[2] isn't used according to 3dmoo's implementation.
-        cmd_buff[3] = archive_handle;
-    } else {
         ERROR_LOG(KERNEL, "failed to get a handle for archive");
-        // TODO(Link Mauve): check for the actual error values, this one was just chosen arbitrarily.
+        // TODO(Link Mauve): Check for the actual error values, this one was just chosen arbitrarily
         cmd_buff[1] = -1;
         return;
     }
 
-    FileSys::Path file_path(filename_type, filename_size, filename_ptr);
+    Handle handle = Kernel::OpenFileFromArchive(archive_handle, file_path, mode);
-    std::string file_string;
-    switch (file_path.GetType()) {
-    case FileSys::Char:
-    case FileSys::Wchar:
-        file_string = file_path.AsString();
-        break;
-    default:
-        WARN_LOG(KERNEL, "file LowPath type is currently unsupported; returning archive handle instead");
-        return;
-    }
-
-    Handle handle = Kernel::OpenFileFromArchive(archive_handle, file_string, mode);
     if (handle) {
         cmd_buff[1] = 0;
         cmd_buff[3] = handle;
     } else {
-        ERROR_LOG(KERNEL, "failed to get a handle for file %s", file_string.c_str());
+        ERROR_LOG(KERNEL, "failed to get a handle for file %s", file_path.DebugStr().c_str());
         // TODO(Link Mauve): check for the actual error values, this one was just chosen arbitrarily.
         cmd_buff[1] = -1;
     }
@@ -163,7 +172,7 @@ void CreateDirectory(Service::Interface* self) {
 
     DEBUG_LOG(KERNEL, "type=%d size=%d data=%s", dirname_type, dirname_size, dir_string.c_str());
 
-    cmd_buff[1] = Kernel::CreateDirectoryFromArchive(archive_handle, dir_string);
+    cmd_buff[1] = Kernel::CreateDirectoryFromArchive(archive_handle, dir_path);
 
     DEBUG_LOG(KERNEL, "called");
 }
@@ -192,7 +201,7 @@ void OpenDirectory(Service::Interface* self) {
 
     DEBUG_LOG(KERNEL, "type=%d size=%d data=%s", dirname_type, dirname_size, dir_string.c_str());
 
-    Handle handle = Kernel::OpenDirectoryFromArchive(archive_handle, dir_string);
+    Handle handle = Kernel::OpenDirectoryFromArchive(archive_handle, dir_path);
     if (handle) {
         cmd_buff[1] = 0;
         cmd_buff[3] = handle;
@@ -205,6 +214,19 @@ void OpenDirectory(Service::Interface* self) {
     DEBUG_LOG(KERNEL, "called");
 }
 
+/**
+ * FS_User::OpenArchive service function
+ *  Inputs:
+ *      1 : Archive ID
+ *      2 : Archive low path type
+ *      3 : Archive low path size
+ *      4 : (LowPathSize << 14) | 2
+ *      5 : Archive low path
+ *  Outputs:
+ *      1 : Result of function, 0 on success, otherwise error code
+ *      2 : Archive handle lower word (unused)
+ *      3 : Archive handle upper word (same as file handle)
+ */
 void OpenArchive(Service::Interface* self) {
     u32* cmd_buff = Service::GetCommandBuffer();
 
@@ -212,10 +234,11 @@ void OpenArchive(Service::Interface* self) {
     auto archivename_type = static_cast<FileSys::LowPathType>(cmd_buff[2]);
     u32 archivename_size  = cmd_buff[3];
     u32 archivename_ptr   = cmd_buff[5];
+    FileSys::Path archive_path(archivename_type, archivename_size, archivename_ptr);
 
-    DEBUG_LOG(KERNEL, "type=%d size=%d", archivename_type, archivename_size);
+    DEBUG_LOG(KERNEL, "archive_path=%s", archive_path.DebugStr().c_str());
 
-    if (archivename_type != FileSys::Empty) {
+    if (archive_path.GetType() != FileSys::Empty) {
         ERROR_LOG(KERNEL, "archive LowPath type other than empty is currently unsupported");
         cmd_buff[1] = -1;
         return;