HLE: Revamp error handling throrough the HLE code

All service calls in the CTR OS return result codes indicating the
success or failure of the call. Previous to this commit, Citra's HLE
emulation of services and the kernel universally either ignored errors
or returned dummy -1 error codes.

This commit makes an initial effort to provide an infrastructure for
error reporting and propagation which can be use going forward to make
HLE calls accurately return errors as the original system. A few parts
of the code have been updated to use the new system where applicable.

One part of this effort is the definition of the `ResultCode` type,
which provides facilities for constructing and parsing error codes in
the structured format used by the CTR.

The `ResultVal` type builds on `ResultCode` by providing a container for
values returned by function that can report errors. It enforces that
correct error checking will be done on function returns by preventing
the use of the return value if the function returned an error code.

Currently this change is mostly internal since errors are still
suppressed on the ARM<->HLE border, as a temporary compatibility hack.
As functionality is implemented and tested this hack can be eventually
removed.

6 files changed, 46 insertions, 62 deletions

diff --git a/src/core/hle/service/fs_user.cpp b/src/core/hle/service/fs_user.cpp
index 2aec1a52..435be5b5 100644
--- a/src/core/hle/service/fs_user.cpp
+++ b/src/core/hle/service/fs_user.cpp
@@ -4,26 +4,24 @@
 
 #include "common/common.h"
 
-#include "fs_user.h"
 #include "common/string_util.h"
-#include "core/settings.h"
 #include "core/hle/kernel/archive.h"
+#include "core/hle/kernel/archive.h"
+#include "core/hle/result.h"
+#include "core/hle/service/fs_user.h"
+#include "core/settings.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // Namespace FS_User
 
 namespace FS_User {
 
-// We currently return 0 for success and -1 for failure in cmd_buff[1].  -1 was chosen because it
-// puts all the sections of the http://3dbrew.org/wiki/Error_codes to something non-zero, to make
-// sure we don't mislead the application into thinking something worked.
-
 static void Initialize(Service::Interface* self) {
     u32* cmd_buff = Service::GetCommandBuffer();
 
     // TODO(Link Mauve): check the behavior when cmd_buff[1] isn't 32, as per
     // http://3dbrew.org/wiki/FS:Initialize#Request
-    cmd_buff[1] = 0;
+    cmd_buff[1] = RESULT_SUCCESS.raw;
 
     DEBUG_LOG(KERNEL, "called");
 }
@@ -59,14 +57,12 @@ static void OpenFile(Service::Interface* self) {
 
     DEBUG_LOG(KERNEL, "path=%s, mode=%d attrs=%d", file_path.DebugStr().c_str(), mode, attributes);
 
-    Handle handle = Kernel::OpenFileFromArchive(archive_handle, file_path, mode);
-    if (handle) {
-        cmd_buff[1] = 0;
-        cmd_buff[3] = handle;
+    ResultVal<Handle> handle = Kernel::OpenFileFromArchive(archive_handle, file_path, mode);
+    cmd_buff[1] = handle.Code().raw;
+    if (handle.Succeeded()) {
+        cmd_buff[3] = *handle;
     } else {
         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;
     }
 
     DEBUG_LOG(KERNEL, "called");
@@ -111,27 +107,27 @@ static void OpenFileDirectly(Service::Interface* self) {
 
     if (archive_path.GetType() != FileSys::Empty) {
         ERROR_LOG(KERNEL, "archive LowPath type other than empty is currently unsupported");
-        cmd_buff[1] = -1;
+        cmd_buff[1] = UnimplementedFunction(ErrorModule::FS).raw;
         return;
     }
 
     // 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) {
+    // TODO(yuriks): Why is there all this duplicate (and seemingly useless) code up here?
+    ResultVal<Handle> archive_handle = Kernel::OpenArchive(archive_id);
+    cmd_buff[1] = archive_handle.Code().raw;
+    if (archive_handle.Failed()) {
         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
-        cmd_buff[1] = -1;
         return;
     }
+    // cmd_buff[2] isn't used according to 3dmoo's implementation.
+    cmd_buff[3] = *archive_handle;
 
-    Handle handle = Kernel::OpenFileFromArchive(archive_handle, file_path, mode);
-    if (handle) {
-        cmd_buff[1] = 0;
-        cmd_buff[3] = handle;
+    ResultVal<Handle> handle = Kernel::OpenFileFromArchive(*archive_handle, file_path, mode);
+    cmd_buff[1] = handle.Code().raw;
+    if (handle.Succeeded()) {
+        cmd_buff[3] = *handle;
     } else {
         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;
     }
 
     DEBUG_LOG(KERNEL, "called");
@@ -243,14 +239,12 @@ static void OpenDirectory(Service::Interface* self) {
 
     DEBUG_LOG(KERNEL, "type=%d size=%d data=%s", dirname_type, dirname_size, dir_path.DebugStr().c_str());
 
-    Handle handle = Kernel::OpenDirectoryFromArchive(archive_handle, dir_path);
-    if (handle) {
-        cmd_buff[1] = 0;
-        cmd_buff[3] = handle;
+    ResultVal<Handle> handle = Kernel::OpenDirectoryFromArchive(archive_handle, dir_path);
+    cmd_buff[1] = handle.Code().raw;
+    if (handle.Succeeded()) {
+        cmd_buff[3] = *handle;
     } else {
         ERROR_LOG(KERNEL, "failed to get a handle for directory");
-        // TODO(Link Mauve): check for the actual error values, this one was just chosen arbitrarily.
-        cmd_buff[1] = -1;
     }
 
     DEBUG_LOG(KERNEL, "called");
@@ -282,19 +276,17 @@ static void OpenArchive(Service::Interface* self) {
 
     if (archive_path.GetType() != FileSys::Empty) {
         ERROR_LOG(KERNEL, "archive LowPath type other than empty is currently unsupported");
-        cmd_buff[1] = -1;
+        cmd_buff[1] = UnimplementedFunction(ErrorModule::FS).raw;
         return;
     }
 
-    Handle handle = Kernel::OpenArchive(archive_id);
-    if (handle) {
-        cmd_buff[1] = 0;
+    ResultVal<Handle> handle = Kernel::OpenArchive(archive_id);
+    cmd_buff[1] = handle.Code().raw;
+    if (handle.Succeeded()) {
         // cmd_buff[2] isn't used according to 3dmoo's implementation.
-        cmd_buff[3] = handle;
+        cmd_buff[3] = *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.
-        cmd_buff[1] = -1;
     }
 
     DEBUG_LOG(KERNEL, "called");
diff --git a/src/core/hle/service/gsp_gpu.cpp b/src/core/hle/service/gsp_gpu.cpp
index 66daded9..de1bd3f6 100644
--- a/src/core/hle/service/gsp_gpu.cpp
+++ b/src/core/hle/service/gsp_gpu.cpp
@@ -28,28 +28,23 @@ u32 g_thread_id = 1;            ///< Thread index into interrupt relay queue, 1
 
 /// Gets a pointer to a thread command buffer in GSP shared memory
 static inline u8* GetCommandBuffer(u32 thread_id) {
-    if (0 == g_shared_memory)
-        return nullptr;
-
-    return Kernel::GetSharedMemoryPointer(g_shared_memory,
-        0x800 + (thread_id * sizeof(CommandBuffer)));
+    ResultVal<u8*> ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, 0x800 + (thread_id * sizeof(CommandBuffer)));
+    return ptr.ValueOr(nullptr);
 }
 
 static inline FrameBufferUpdate* GetFrameBufferInfo(u32 thread_id, u32 screen_index) {
-    if (0 == g_shared_memory)
-        return nullptr;
-
     _dbg_assert_msg_(GSP, screen_index < 2, "Invalid screen index");
 
     // For each thread there are two FrameBufferUpdate fields
     u32 offset = 0x200 + (2 * thread_id + screen_index) * sizeof(FrameBufferUpdate);
-    return (FrameBufferUpdate*)Kernel::GetSharedMemoryPointer(g_shared_memory, offset);
+    ResultVal<u8*> ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, offset);
+    return reinterpret_cast<FrameBufferUpdate*>(ptr.ValueOr(nullptr));
 }
 
 /// Gets a pointer to the interrupt relay queue for a given thread index
 static inline InterruptRelayQueue* GetInterruptRelayQueue(u32 thread_id) {
-    return (InterruptRelayQueue*)Kernel::GetSharedMemoryPointer(g_shared_memory,
-        sizeof(InterruptRelayQueue) * thread_id);
+    ResultVal<u8*> ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, sizeof(InterruptRelayQueue) * thread_id);
+    return reinterpret_cast<InterruptRelayQueue*>(ptr.ValueOr(nullptr));
 }
 
 static void WriteHWRegs(u32 base_address, u32 size_in_bytes, const u32* data) {
diff --git a/src/core/hle/service/hid_user.cpp b/src/core/hle/service/hid_user.cpp
index 5f6bf1ef..d29de1a5 100644
--- a/src/core/hle/service/hid_user.cpp
+++ b/src/core/hle/service/hid_user.cpp
@@ -34,10 +34,7 @@ static s16 next_circle_y = 0;
  * Gets a pointer to the PadData structure inside HID shared memory
  */
 static inline PadData* GetPadData() {
-    if (0 == shared_mem)
-        return nullptr;
-
-    return reinterpret_cast<PadData*>(Kernel::GetSharedMemoryPointer(shared_mem, 0));
+    return reinterpret_cast<PadData*>(Kernel::GetSharedMemoryPointer(shared_mem, 0).ValueOr(nullptr));
 }
 
 /**
diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp
index abc8d5ed..fed2268a 100644
--- a/src/core/hle/service/service.cpp
+++ b/src/core/hle/service/service.cpp
@@ -62,7 +62,7 @@ void Manager::DeleteService(const std::string& port_name) {
 
 /// Get a Service Interface from its Handle
 Interface* Manager::FetchFromHandle(Handle handle) {
-    return Kernel::g_object_pool.GetFast<Interface>(handle);
+    return Kernel::g_object_pool.Get<Interface>(handle);
 }
 
 /// Get a Service Interface from its port
diff --git a/src/core/hle/service/service.h b/src/core/hle/service/service.h
index 55aa84e8..136984b9 100644
--- a/src/core/hle/service/service.h
+++ b/src/core/hle/service/service.h
@@ -80,7 +80,7 @@ public:
      * @param wait Boolean wait set if current thread should wait as a result of sync operation
      * @return Result of operation, 0 on success, otherwise error code
      */
-    Result SyncRequest(bool* wait) override {
+    ResultVal<bool> SyncRequest() override {
         u32* cmd_buff = GetCommandBuffer();
         auto itr = m_functions.find(cmd_buff[0]);
 
@@ -91,7 +91,7 @@ public:
             // TODO(bunnei): Hack - ignore error
             u32* cmd_buff = Service::GetCommandBuffer();
             cmd_buff[1] = 0;
-            return 0;
+            return MakeResult<bool>(false);
         }
         if (itr->second.func == nullptr) {
             ERROR_LOG(OSHLE, "unimplemented function: port=%s, name=%s",
@@ -100,12 +100,12 @@ public:
             // TODO(bunnei): Hack - ignore error
             u32* cmd_buff = Service::GetCommandBuffer();
             cmd_buff[1] = 0;
-            return 0;
+            return MakeResult<bool>(false);
         }
 
         itr->second.func(this);
 
-        return 0; // TODO: Implement return from actual function
+        return MakeResult<bool>(false); // TODO: Implement return from actual function
     }
 
     /**
@@ -113,10 +113,10 @@ public:
      * @param wait Boolean wait set if current thread should wait as a result of sync operation
      * @return Result of operation, 0 on success, otherwise error code
      */
-    Result WaitSynchronization(bool* wait) override {
+    ResultVal<bool> WaitSynchronization() override {
         // TODO(bunnei): ImplementMe
         ERROR_LOG(OSHLE, "unimplemented function");
-        return 0;
+        return UnimplementedFunction(ErrorModule::OS);
     }
 
 protected:
diff --git a/src/core/hle/service/srv.cpp b/src/core/hle/service/srv.cpp
index df38bd93..0e7fa9e3 100644
--- a/src/core/hle/service/srv.cpp
+++ b/src/core/hle/service/srv.cpp
@@ -35,7 +35,7 @@ static void GetProcSemaphore(Service::Interface* self) {
 }
 
 static void GetServiceHandle(Service::Interface* self) {
-    Result res = 0;
+    ResultCode res = RESULT_SUCCESS;
     u32* cmd_buff = Service::GetCommandBuffer();
 
     std::string port_name = std::string((const char*)&cmd_buff[1], 0, Service::kMaxPortSize);
@@ -46,9 +46,9 @@ static void GetServiceHandle(Service::Interface* self) {
         DEBUG_LOG(OSHLE, "called port=%s, handle=0x%08X", port_name.c_str(), cmd_buff[3]);
     } else {
         ERROR_LOG(OSHLE, "(UNIMPLEMENTED) called port=%s", port_name.c_str());
-        res = -1;
+        res = UnimplementedFunction(ErrorModule::SRV);
     }
-    cmd_buff[1] = res;
+    cmd_buff[1] = res.raw;
 }
 
 const Interface::FunctionInfo FunctionTable[] = {