fsp_srv: Various improvements to IStorage:Read implementation.

src/core/hle/ipc_helpers.h

@@ -304,6 +304,11 @@ inline u64 RequestParser::Pop() {
     return msw << 32 | lsw;
 }
 
+template <>
+inline s64 RequestParser::Pop() {
+    return static_cast<s64>(Pop<u64>());
+}
+
 template <>
 inline bool RequestParser::Pop() {
     return Pop<u8>() != 0;

src/core/hle/result.h

@@ -19,6 +19,8 @@
 enum class ErrorDescription : u32 {
     Success = 0,
     RemoteProcessDead = 301,
+    InvalidOffset = 6061,
+    InvalidLength = 6062,
 };
 
 /**

src/core/hle/service/filesystem/filesystem.h

@@ -6,11 +6,20 @@
 
 #include <memory>
 #include "common/common_types.h"
-#include "core/file_sys/filesystem.h"
 #include "core/hle/result.h"
-#include "core/hle/service/service.h"
+
+namespace FileSys {
+class FileSystemBackend;
+class FileSystemFactory;
+class Path;
+} // namespace FileSys
 
 namespace Service {
+
+namespace SM {
+class ServiceManager;
+} // namespace SM
+
 namespace FileSystem {
 
 /// Supported FileSystem types
@@ -37,5 +46,5 @@ ResultVal<std::unique_ptr<FileSys::FileSystemBackend>> OpenFileSystem(Type type,
 /// Registers all Filesystem services with the specified service manager.
 void InstallInterfaces(SM::ServiceManager& service_manager);
 
-} // namespace Filesystem
+} // namespace FileSystem
 } // namespace Service

src/core/hle/service/filesystem/fsp_srv.cpp

@@ -20,9 +20,8 @@ public:
     IStorage(std::unique_ptr<FileSys::StorageBackend>&& backend)
         : ServiceFramework("IStorage"), backend(std::move(backend)) {
         static const FunctionInfo functions[] = {
-            {0, &IStorage::Read, "Read"},       {1, &IStorage::Write, "Write"},
-            {2, &IStorage::Flush, "Flush"},     {3, &IStorage::SetSize, "SetSize"},
-            {4, &IStorage::GetSize, "GetSize"},
+            {0, &IStorage::Read, "Read"}, {1, nullptr, "Write"},   {2, nullptr, "Flush"},
+            {3, nullptr, "SetSize"},      {4, nullptr, "GetSize"},
         };
         RegisterHandlers(functions);
     }
@@ -32,48 +31,39 @@ private:
 
     void Read(Kernel::HLERequestContext& ctx) {
         IPC::RequestParser rp{ctx};
-        u64 offset = rp.Pop<u64>();
-        u64 length = rp.Pop<u64>();
+        const s64 offset = rp.Pop<s64>();
+        const s64 length = rp.Pop<s64>();
+        const auto& descriptor = ctx.BufferDescriptorB()[0];
 
-        LOG_DEBUG(Service, "called, offset=0x%llx, length=0x%llx", offset, length);
+        LOG_DEBUG(Service_FS, "called, offset=0x%llx, length=0x%llx", offset, length);
 
-        auto descriptor = ctx.BufferDescriptorB()[0];
-        std::vector<u8> output(length);
-
-        ResultVal<size_t> res = backend->Read(offset, length, output.data());
-        if (res.Failed()) {
+        // Error checking
+        ASSERT_MSG(length == descriptor.Size(), "unexpected size difference");
+        if (length < 0) {
             IPC::RequestBuilder rb{ctx, 2};
-            rb.Push(res.Code());
-        }
-
-        Memory::WriteBlock(descriptor.Address(), output.data(), descriptor.Size());
-
-        IPC::RequestBuilder rb{ctx, 2};
-        rb.Push(RESULT_SUCCESS);
+            rb.Push(ResultCode(ErrorModule::FS, ErrorDescription::InvalidLength));
+            return;
         }
-
-    void Write(Kernel::HLERequestContext& ctx) {
+        if (offset < 0) {
             IPC::RequestBuilder rb{ctx, 2};
-        rb.Push(RESULT_SUCCESS);
-        LOG_WARNING(Service, "(STUBBED) called");
+            rb.Push(ResultCode(ErrorModule::FS, ErrorDescription::InvalidOffset));
+            return;
         }
 
-    void Flush(Kernel::HLERequestContext& ctx) {
+        // Read the data from the Storage backend
+        std::vector<u8> output(length);
+        ResultVal<size_t> res = backend->Read(offset, length, output.data());
+        if (res.Failed()) {
             IPC::RequestBuilder rb{ctx, 2};
-        rb.Push(RESULT_SUCCESS);
-        LOG_WARNING(Service, "(STUBBED) called");
+            rb.Push(res.Code());
+            return;
         }
 
-    void SetSize(Kernel::HLERequestContext& ctx) {
-        IPC::RequestBuilder rb{ctx, 2};
-        rb.Push(RESULT_SUCCESS);
-        LOG_WARNING(Service, "(STUBBED) called");
-    }
+        // Write the data to memory
+        Memory::WriteBlock(descriptor.Address(), output.data(), descriptor.Size());
 
-    void GetSize(Kernel::HLERequestContext& ctx) {
         IPC::RequestBuilder rb{ctx, 2};
         rb.Push(RESULT_SUCCESS);
-        LOG_WARNING(Service, "(STUBBED) called");
     }
 };
 
@@ -87,46 +77,62 @@ FSP_SRV::FSP_SRV() : ServiceFramework("fsp-srv") {
     RegisterHandlers(functions);
 }
 
+void FSP_SRV::TryLoadRomFS() {
+    if (romfs) {
+        return;
+    }
+    FileSys::Path unused;
+    auto res = OpenFileSystem(Type::RomFS, unused);
+    if (res.Succeeded()) {
+        romfs = std::move(res.Unwrap());
+    }
+}
+
 void FSP_SRV::Initalize(Kernel::HLERequestContext& ctx) {
+    LOG_WARNING(Service_FS, "(STUBBED) called");
+
     IPC::RequestBuilder rb{ctx, 2};
     rb.Push(RESULT_SUCCESS);
-    LOG_WARNING(Service, "(STUBBED) called");
 }
 
 void FSP_SRV::GetGlobalAccessLogMode(Kernel::HLERequestContext& ctx) {
+    LOG_WARNING(Service_FS, "(STUBBED) called");
+
     IPC::RequestBuilder rb{ctx, 4};
     rb.Push(RESULT_SUCCESS);
     rb.Push<u32>(5);
-    LOG_WARNING(Service, "(STUBBED) called");
 }
 
 void FSP_SRV::OpenDataStorageByCurrentProcess(Kernel::HLERequestContext& ctx) {
-    FileSys::Path path;
-    auto filesystem = OpenFileSystem(Type::RomFS, path);
-    if (filesystem.Failed()) {
+    LOG_DEBUG(Service_FS, "called");
+
+    TryLoadRomFS();
+    if (!romfs) {
+        // TODO (bunnei): Find the right error code to use here
+        LOG_CRITICAL(Service_FS, "no file system interface available!");
         IPC::RequestBuilder rb{ctx, 2};
-        rb.Push(filesystem.Code());
+        rb.Push(ResultCode(-1));
         return;
     }
 
-    auto storage = filesystem.Unwrap()->OpenFile({}, {});
+    // Attempt to open a StorageBackend interface to the RomFS
+    auto storage = romfs->OpenFile({}, {});
     if (storage.Failed()) {
+        LOG_CRITICAL(Service_FS, "no storage interface available!");
         IPC::RequestBuilder rb{ctx, 2};
         rb.Push(storage.Code());
         return;
     }
 
-    // TODO: What if already opened?
-
     IPC::RequestBuilder rb{ctx, 2, 0, 0, 1};
     rb.Push(RESULT_SUCCESS);
     rb.PushIpcInterface<IStorage>(std::move(storage.Unwrap()));
-    LOG_WARNING(Service, "(STUBBED) called");
 }
 
 void FSP_SRV::OpenRomStorage(Kernel::HLERequestContext& ctx) {
+    LOG_WARNING(Service_FS, "(STUBBED) called, using OpenDataStorageByCurrentProcess");
     OpenDataStorageByCurrentProcess(ctx);
 }
 
-} // namespace Filesystem
+} // namespace FileSystem
 } // namespace Service

src/core/hle/service/filesystem/fsp_srv.h

@@ -4,22 +4,31 @@
 
 #pragma once
 
+#include <memory>
 #include "core/hle/service/service.h"
 
+namespace FileSys {
+class FileSystemBackend;
+}
+
 namespace Service {
 namespace FileSystem {
 
 class FSP_SRV final : public ServiceFramework<FSP_SRV> {
 public:
-    FSP_SRV();
+    explicit FSP_SRV();
     ~FSP_SRV() = default;
 
 private:
+    void TryLoadRomFS();
+
     void Initalize(Kernel::HLERequestContext& ctx);
     void GetGlobalAccessLogMode(Kernel::HLERequestContext& ctx);
     void OpenDataStorageByCurrentProcess(Kernel::HLERequestContext& ctx);
     void OpenRomStorage(Kernel::HLERequestContext& ctx);
+
+    std::unique_ptr<FileSys::FileSystemBackend> romfs;
 };
 
-} // namespace Filesystem
+} // namespace FileSystem
 } // namespace Service