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.

src/core/CMakeLists.txt

@@ -133,6 +133,7 @@ set(HEADERS
             hle/service/srv.h
             hle/service/ssl_c.h
             hle/config_mem.h
+            hle/result.h
             hle/function_wrappers.h
             hle/hle.h
             hle/svc.h

src/core/arm/skyeye_common/armdefs.h

@@ -799,22 +799,24 @@ pascal void SpinCursor (short increment);    /* copied from CursorCtl.h */
 #include "list.h"
 #include "tb.h"
 */
-const int EQ = 0;
+enum ConditionCode {
-const int NE = 1;
+    EQ = 0,
-const int CS = 2;
+    NE = 1,
-const int CC = 3;
+    CS = 2,
-const int MI = 4;
+    CC = 3,
-const int PL = 5;
+    MI = 4,
-const int VS = 6;
+    PL = 5,
-const int VC = 7;
+    VS = 6,
-const int HI = 8;
+    VC = 7,
-const int LS = 9;
+    HI = 8,
-const int GE = 10;
+    LS = 9,
-const int LT = 11;
+    GE = 10,
-const int GT = 12;
+    LT = 11,
-const int LE = 13;
+    GT = 12,
-const int AL = 14;
+    LE = 13,
-const int NV = 15;
+    AL = 14,
+    NV = 15,
+};
 
 #ifndef NFLAG
 #define NFLAG    state->NFlag

src/core/hle/kernel/address_arbiter.cpp

@@ -30,17 +30,17 @@ 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)");
-        return 0;
+        return UnimplementedFunction(ErrorModule::OS);
     }
 };
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
 /// Arbitrate an address
-Result ArbitrateAddress(Handle handle, ArbitrationType type, u32 address, s32 value) {
+ResultCode ArbitrateAddress(Handle handle, ArbitrationType type, u32 address, s32 value) {
     switch (type) {
 
     // Signal thread(s) waiting for arbitrate address...
@@ -65,9 +65,9 @@ Result ArbitrateAddress(Handle handle, ArbitrationType type, u32 address, s32 va
 
     default:
         ERROR_LOG(KERNEL, "unknown type=%d", type);
-        return -1;
+        return ResultCode(ErrorDescription::InvalidEnumValue, ErrorModule::Kernel, ErrorSummary::WrongArgument, ErrorLevel::Usage);
     }
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /// Create an address arbiter

src/core/hle/kernel/address_arbiter.h

@@ -28,7 +28,7 @@ enum class ArbitrationType : u32 {
 };
 
 /// Arbitrate an address
-Result ArbitrateAddress(Handle handle, ArbitrationType type, u32 address, s32 value);
+ResultCode ArbitrateAddress(Handle handle, ArbitrationType type, u32 address, s32 value);
 
 /// Create an address arbiter
 Handle CreateAddressArbiter(const std::string& name = "Unknown");

src/core/hle/kernel/archive.cpp

@@ -9,8 +9,9 @@
 #include "core/file_sys/archive.h"
 #include "core/file_sys/archive_sdmc.h"
 #include "core/file_sys/directory.h"
-#include "core/hle/service/service.h"
 #include "core/hle/kernel/archive.h"
+#include "core/hle/result.h"
+#include "core/hle/service/service.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // Kernel namespace
@@ -56,7 +57,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 = Service::GetCommandBuffer();
         FileCommand cmd = static_cast<FileCommand>(cmd_buff[0]);
 
@@ -106,11 +107,11 @@ public:
         default:
         {
             ERROR_LOG(KERNEL, "Unknown command=0x%08X!", cmd);
-            return -1;
+            return UnimplementedFunction(ErrorModule::FS);
         }
         }
         cmd_buff[1] = 0; // No error
-        return 0;
+        return MakeResult<bool>(false);
     }
 
     /**
@@ -118,10 +119,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)");
-        return 0;
+        return UnimplementedFunction(ErrorModule::FS);
     }
 };
 
@@ -141,7 +142,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 = Service::GetCommandBuffer();
         FileCommand cmd = static_cast<FileCommand>(cmd_buff[0]);
         switch (cmd) {
@@ -183,7 +184,8 @@ public:
         case FileCommand::SetSize:
         {
             u64 size = cmd_buff[1] | ((u64)cmd_buff[2] << 32);
-            DEBUG_LOG(KERNEL, "SetSize %s %s size=%llu", GetTypeName().c_str(), GetName().c_str(), size);
+            DEBUG_LOG(KERNEL, "SetSize %s %s size=%llu",
+                    GetTypeName().c_str(), GetName().c_str(), size);
             backend->SetSize(size);
             break;
         }
@@ -198,11 +200,12 @@ public:
         // Unknown command...
         default:
             ERROR_LOG(KERNEL, "Unknown command=0x%08X!", cmd);
-            cmd_buff[1] = -1; // TODO(Link Mauve): use the correct error code for that.
+            ResultCode error = UnimplementedFunction(ErrorModule::FS);
-            return -1;
+            cmd_buff[1] = error.raw; // TODO(Link Mauve): use the correct error code for that.
+            return error;
         }
         cmd_buff[1] = 0; // No error
-        return 0;
+        return MakeResult<bool>(false);
     }
 
     /**
@@ -210,10 +213,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)");
-        return 0;
+        return UnimplementedFunction(ErrorModule::FS);
     }
 };
 
@@ -233,7 +236,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 = Service::GetCommandBuffer();
         DirectoryCommand cmd = static_cast<DirectoryCommand>(cmd_buff[0]);
         switch (cmd) {
@@ -243,8 +246,9 @@ public:
         {
             u32 count = cmd_buff[1];
             u32 address = cmd_buff[3];
-            FileSys::Entry* entries = reinterpret_cast<FileSys::Entry*>(Memory::GetPointer(address));
+            auto entries = reinterpret_cast<FileSys::Entry*>(Memory::GetPointer(address));
-            DEBUG_LOG(KERNEL, "Read %s %s: count=%d", GetTypeName().c_str(), GetName().c_str(), count);
+            DEBUG_LOG(KERNEL, "Read %s %s: count=%d",
+                    GetTypeName().c_str(), GetName().c_str(), count);
 
             // Number of entries actually read
             cmd_buff[2] = backend->Read(count, entries);
@@ -261,11 +265,12 @@ public:
         // Unknown command...
         default:
             ERROR_LOG(KERNEL, "Unknown command=0x%08X!", cmd);
-            cmd_buff[1] = -1; // TODO(Link Mauve): use the correct error code for that.
+            ResultCode error = UnimplementedFunction(ErrorModule::FS);
-            return -1;
+            cmd_buff[1] = error.raw; // TODO(Link Mauve): use the correct error code for that.
+            return error;
         }
         cmd_buff[1] = 0; // No error
-        return 0;
+        return MakeResult<bool>(false);
     }
 
     /**
@@ -273,10 +278,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)");
-        return 0;
+        return UnimplementedFunction(ErrorModule::FS);
     }
 };
 
@@ -284,89 +289,59 @@ public:
 
 std::map<FileSys::Archive::IdCode, Handle> g_archive_map; ///< Map of file archives by IdCode
 
-/**
+ResultVal<Handle> OpenArchive(FileSys::Archive::IdCode id_code) {
- * Opens an archive
- * @param id_code IdCode of the archive to open
- * @return Handle to archive if it exists, otherwise a null handle (0)
- */
-Handle OpenArchive(FileSys::Archive::IdCode id_code) {
     auto itr = g_archive_map.find(id_code);
     if (itr == g_archive_map.end()) {
-        return 0;
+        return ResultCode(ErrorDescription::NotFound, ErrorModule::FS,
-    }
+                ErrorSummary::NotFound, ErrorLevel::Permanent);
-    return itr->second;
     }
 
-/**
+    return MakeResult<Handle>(itr->second);
- * Closes an archive
+}
- * @param id_code IdCode of the archive to open
+
- * @return Result of operation, 0 on success, otherwise error code
+ResultCode CloseArchive(FileSys::Archive::IdCode id_code) {
- */
-Result CloseArchive(FileSys::Archive::IdCode id_code) {
     auto itr = g_archive_map.find(id_code);
     if (itr == g_archive_map.end()) {
         ERROR_LOG(KERNEL, "Cannot close archive %d, does not exist!", (int)id_code);
-        return -1;
+        return InvalidHandle(ErrorModule::FS);
     }
 
     INFO_LOG(KERNEL, "Closed archive %d", (int) id_code);
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**
  * Mounts an archive
  * @param archive Pointer to the archive to mount
- * @return Result of operation, 0 on success, otherwise error code
+ * @return Result of operation
  */
-Result MountArchive(Archive* archive) {
+ResultCode MountArchive(Archive* archive) {
     FileSys::Archive::IdCode id_code = archive->backend->GetIdCode();
-    if (0 != OpenArchive(id_code)) {
+    ResultVal<Handle> archive_handle = OpenArchive(id_code);
+    if (archive_handle.Succeeded()) {
         ERROR_LOG(KERNEL, "Cannot mount two archives with the same ID code! (%d)", (int) id_code);
-        return -1;
+        return archive_handle.Code();
     }
     g_archive_map[id_code] = archive->GetHandle();
     INFO_LOG(KERNEL, "Mounted archive %s", archive->GetName().c_str());
-    return 0;
+    return RESULT_SUCCESS;
 }
 
-/**
+ResultCode CreateArchive(FileSys::Archive* backend, const std::string& name) {
- * Creates an Archive
- * @param handle Handle to newly created archive object
- * @param backend File system backend interface to the archive
- * @param name Optional name of Archive
- * @return Newly created Archive object
- */
-Archive* CreateArchive(Handle& handle, FileSys::Archive* backend, const std::string& name) {
     Archive* archive = new Archive;
-    handle = Kernel::g_object_pool.Create(archive);
+    Handle handle = Kernel::g_object_pool.Create(archive);
     archive->name = name;
     archive->backend = backend;
 
-    MountArchive(archive);
+    ResultCode result = MountArchive(archive);
-
+    if (result.IsError()) {
-    return archive;
+        return result;
     }
     
-/**
+    return RESULT_SUCCESS;
- * Creates an Archive
- * @param backend File system backend interface to the archive
- * @param name Optional name of Archive
- * @return Handle to newly created Archive object
- */
-Handle CreateArchive(FileSys::Archive* backend, const std::string& name) {
-    Handle handle;
-    CreateArchive(handle, backend, name);
-    return handle;
 }
 
-/**
+ResultVal<Handle> OpenFileFromArchive(Handle archive_handle, const FileSys::Path& path, const FileSys::Mode mode) {
- * Open a File from an Archive
- * @param archive_handle Handle to an open Archive object
- * @param path Path to the File inside of the Archive
- * @param mode Mode under which to open the File
- * @return Opened File object
- */
-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.
@@ -376,19 +351,24 @@ Handle OpenFileFromArchive(Handle archive_handle, const FileSys::Path& path, con
         // 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;
+        return MakeResult<Handle>(archive_handle);
 
     File* file = new File;
     Handle handle = Kernel::g_object_pool.Create(file);
 
-    Archive* archive = Kernel::g_object_pool.GetFast<Archive>(archive_handle);
+    Archive* archive = Kernel::g_object_pool.Get<Archive>(archive_handle);
+    if (archive == nullptr) {
+        return InvalidHandle(ErrorModule::FS);
+    }
     file->path = path;
     file->backend = archive->backend->OpenFile(path, mode);
 
-    if (!file->backend)
+    if (!file->backend) {
-        return 0;
+        return ResultCode(ErrorDescription::NotFound, ErrorModule::FS,
+                ErrorSummary::NotFound, ErrorLevel::Permanent);
+    }
 
-    return handle;
+    return MakeResult<Handle>(handle);
 }
 
 /**
@@ -442,15 +422,18 @@ Result CreateDirectoryFromArchive(Handle archive_handle, const FileSys::Path& pa
  * @param path Path to the Directory inside of the Archive
  * @return Opened Directory object
  */
-Handle OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path) {
+ResultVal<Handle> OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path) {
     Directory* directory = new Directory;
     Handle handle = Kernel::g_object_pool.Create(directory);
 
-    Archive* archive = Kernel::g_object_pool.GetFast<Archive>(archive_handle);
+    Archive* archive = Kernel::g_object_pool.Get<Archive>(archive_handle);
+    if (archive == nullptr) {
+        return InvalidHandle(ErrorModule::FS);
+    }
     directory->path = path;
     directory->backend = archive->backend->OpenDirectory(path);
 
-    return handle;
+    return MakeResult<Handle>(handle);
 }
 
 /// Initialize archives

src/core/hle/kernel/archive.h

@@ -6,8 +6,9 @@
 
 #include "common/common_types.h"
 
-#include "core/hle/kernel/kernel.h"
 #include "core/file_sys/archive.h"
+#include "core/hle/kernel/kernel.h"
+#include "core/hle/result.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // Kernel namespace
@@ -17,33 +18,31 @@ namespace Kernel {
 /**
  * Opens an archive
  * @param id_code IdCode of the archive to open
- * @return Handle to archive if it exists, otherwise a null handle (0)
+ * @return Handle to the opened archive
  */
-Handle OpenArchive(FileSys::Archive::IdCode id_code);
+ResultVal<Handle> OpenArchive(FileSys::Archive::IdCode id_code);
 
 /**
  * Closes an archive
  * @param id_code IdCode of the archive to open
- * @return true if it worked fine
  */
-Result CloseArchive(FileSys::Archive::IdCode id_code);
+ResultCode CloseArchive(FileSys::Archive::IdCode id_code);
 
 /**
  * Creates an Archive
  * @param backend File system backend interface to the archive
- * @param name Optional name of Archive
+ * @param name Name of Archive
- * @return Handle to newly created Archive object
  */
-Handle CreateArchive(FileSys::Archive* backend, const std::string& name);
+ResultCode CreateArchive(FileSys::Archive* backend, const std::string& name);
 
 /**
  * Open a File from an Archive
  * @param archive_handle Handle to an open Archive object
  * @param path Path to the File inside of the Archive
  * @param mode Mode under which to open the File
- * @return Opened File object
+ * @return Handle to the opened File object
  */
-Handle OpenFileFromArchive(Handle archive_handle, const FileSys::Path& path, const FileSys::Mode mode);
+ResultVal<Handle> OpenFileFromArchive(Handle archive_handle, const FileSys::Path& path, const FileSys::Mode mode);
 
 /**
  * Delete a File from an Archive
@@ -73,9 +72,9 @@ Result CreateDirectoryFromArchive(Handle archive_handle, const FileSys::Path& pa
  * Open a Directory from an Archive
  * @param archive_handle Handle to an open Archive object
  * @param path Path to the Directory inside of the Archive
- * @return Opened Directory object
+ * @return Handle to the opened File object
  */
-Handle OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path);
+ResultVal<Handle> OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path);
 
 /// Initialize archives
 void ArchiveInit();

src/core/hle/kernel/event.cpp

@@ -35,8 +35,8 @@ 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 {
-        *wait = locked;
+        bool wait = locked;
         if (locked) {
             Handle thread = GetCurrentThreadHandle();
             if (std::find(waiting_threads.begin(), waiting_threads.end(), thread) == waiting_threads.end()) {
@@ -47,7 +47,7 @@ public:
         if (reset_type != RESETTYPE_STICKY && !permanent_locked) {
             locked = true;
         }
-        return 0;
+        return MakeResult<bool>(wait);
     }
 };
 
@@ -57,12 +57,12 @@ public:
  * @param permanent_locked Boolean permanent locked value to set event
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result SetPermanentLock(Handle handle, const bool permanent_locked) {
+ResultCode SetPermanentLock(Handle handle, const bool permanent_locked) {
-    Event* evt = g_object_pool.GetFast<Event>(handle);
+    Event* evt = g_object_pool.Get<Event>(handle);
-    _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!");
+    if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     evt->permanent_locked = permanent_locked;
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**
@@ -71,14 +71,14 @@ Result SetPermanentLock(Handle handle, const bool permanent_locked) {
  * @param locked Boolean locked value to set event
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result SetEventLocked(const Handle handle, const bool locked) {
+ResultCode SetEventLocked(const Handle handle, const bool locked) {
-    Event* evt = g_object_pool.GetFast<Event>(handle);
+    Event* evt = g_object_pool.Get<Event>(handle);
-    _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!");
+    if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     if (!evt->permanent_locked) {
         evt->locked = locked;
     }
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**
@@ -86,9 +86,9 @@ Result SetEventLocked(const Handle handle, const bool locked) {
  * @param handle Handle to event to signal
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result SignalEvent(const Handle handle) {
+ResultCode SignalEvent(const Handle handle) {
-    Event* evt = g_object_pool.GetFast<Event>(handle);
+    Event* evt = g_object_pool.Get<Event>(handle);
-    _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!");
+    if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     // Resume threads waiting for event to signal
     bool event_caught = false;
@@ -106,7 +106,7 @@ Result SignalEvent(const Handle handle) {
     if (!evt->permanent_locked) {
         evt->locked = event_caught;
     }
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**
@@ -114,14 +114,14 @@ Result SignalEvent(const Handle handle) {
  * @param handle Handle to event to clear
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result ClearEvent(Handle handle) {
+ResultCode ClearEvent(Handle handle) {
-    Event* evt = g_object_pool.GetFast<Event>(handle);
+    Event* evt = g_object_pool.Get<Event>(handle);
-    _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!");
+    if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     if (!evt->permanent_locked) {
         evt->locked = true;
     }
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**

src/core/hle/kernel/event.h

@@ -17,7 +17,7 @@ namespace Kernel {
  * @param locked Boolean locked value to set event
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result SetEventLocked(const Handle handle, const bool locked);
+ResultCode SetEventLocked(const Handle handle, const bool locked);
 
 /**
  * Hackish function to set an events permanent lock state, used to pass through synch blocks
@@ -25,21 +25,21 @@ Result SetEventLocked(const Handle handle, const bool locked);
  * @param permanent_locked Boolean permanent locked value to set event
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result SetPermanentLock(Handle handle, const bool permanent_locked);
+ResultCode SetPermanentLock(Handle handle, const bool permanent_locked);
 
 /**
  * Signals an event
  * @param handle Handle to event to signal
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result SignalEvent(const Handle handle);
+ResultCode SignalEvent(const Handle handle);
 
 /**
  * Clears an event
  * @param handle Handle to event to clear
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result ClearEvent(Handle handle);
+ResultCode ClearEvent(Handle handle);
 
 /**
  * Creates an event

src/core/hle/kernel/kernel.h

@@ -7,6 +7,7 @@
 #include <array>
 #include <string>
 #include "common/common.h"
+#include "core/hle/result.h"
 
 typedef u32 Handle;
 typedef s32 Result;
@@ -52,21 +53,19 @@ public:
     virtual Kernel::HandleType GetHandleType() const = 0;
 
     /**
-     * Synchronize kernel object
+     * Synchronize kernel object.
-     * @param wait Boolean wait set if current thread should wait as a result of sync operation
+     * @return True if the current thread should wait as a result of the sync
-     * @return Result of operation, 0 on success, otherwise error code
      */
-    virtual Result SyncRequest(bool* wait) {
+    virtual ResultVal<bool> SyncRequest() {
         ERROR_LOG(KERNEL, "(UNIMPLEMENTED)");
-        return -1;
+        return UnimplementedFunction(ErrorModule::Kernel);
     }
 
     /**
-     * Wait for kernel object to synchronize
+     * Wait for kernel object to synchronize.
-     * @param wait Boolean wait set if current thread should wait as a result of sync operation
+     * @return True if the current thread should wait as a result of the wait
-     * @return Result of operation, 0 on success, otherwise error code
      */
-    virtual Result WaitSynchronization(bool* wait) = 0;
+    virtual ResultVal<bool> WaitSynchronization() = 0;
 };
 
 class ObjectPool : NonCopyable {
@@ -80,38 +79,29 @@ public:
     static Object* CreateByIDType(int type);
 
     template <class T>
-    u32 Destroy(Handle handle) {
+    void Destroy(Handle handle) {
-        u32 error;
+        if (Get<T>(handle)) {
-        if (Get<T>(handle, error)) {
             occupied[handle - HANDLE_OFFSET] = false;
             delete pool[handle - HANDLE_OFFSET];
         }
-        return error;
     }
 
     bool IsValid(Handle handle);
 
     template <class T>
-    T* Get(Handle handle, u32& outError) {
+    T* Get(Handle handle) {
         if (handle < HANDLE_OFFSET || handle >= HANDLE_OFFSET + MAX_COUNT || !occupied[handle - HANDLE_OFFSET]) {
-            // Tekken 6 spams 0x80020001 gets wrong with no ill effects, also on the real PSP
+            if (handle != 0) {
-            if (handle != 0 && (u32)handle != 0x80020001) {
                 WARN_LOG(KERNEL, "Kernel: Bad object handle %i (%08x)", handle, handle);
             }
-            outError = 0;//T::GetMissingErrorCode();
+            return nullptr;
-            return 0;
         } else {
-            // Previously we had a dynamic_cast here, but since RTTI was disabled traditionally,
+            Object* t = pool[handle - HANDLE_OFFSET];
-            // it just acted as a static case and everything worked. This means that we will never
+            if (t->GetHandleType() != T::GetStaticHandleType()) {
-            // see the Wrong type object error below, but we'll just have to live with that danger.
-            T* t = static_cast<T*>(pool[handle - HANDLE_OFFSET]);
-            if (t == 0 || t->GetHandleType() != T::GetStaticHandleType()) {
                 WARN_LOG(KERNEL, "Kernel: Wrong object type for %i (%08x)", handle, handle);
-                outError = 0;//T::GetMissingErrorCode();
+                return nullptr;
-                return 0;
             }
-            outError = 0;//SCE_KERNEL_ERROR_OK;
+            return static_cast<T*>(t);
-            return t;
         }
     }
 

src/core/hle/kernel/mutex.cpp

@@ -32,10 +32,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 SyncRequest(bool* wait) override {
+    ResultVal<bool> SyncRequest() override {
         // TODO(bunnei): ImplementMe
         locked = true;
-        return 0;
+        return MakeResult<bool>(false);
     }
 
     /**
@@ -43,15 +43,14 @@ 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
-        *wait = locked;
+        bool wait = locked;
-
         if (locked) {
             Kernel::WaitCurrentThread(WAITTYPE_MUTEX, GetHandle());
         }
 
-        return 0;
+        return MakeResult<bool>(wait);
     }
 };
 
@@ -119,15 +118,17 @@ bool ReleaseMutex(Mutex* mutex) {
  * Releases a mutex
  * @param handle Handle to mutex to release
  */
-Result ReleaseMutex(Handle handle) {
+ResultCode ReleaseMutex(Handle handle) {
-    Mutex* mutex = Kernel::g_object_pool.GetFast<Mutex>(handle);
+    Mutex* mutex = Kernel::g_object_pool.Get<Mutex>(handle);
-
+    if (mutex == nullptr) return InvalidHandle(ErrorModule::Kernel);
-    _assert_msg_(KERNEL, (mutex != nullptr), "ReleaseMutex tried to release a nullptr mutex!");
 
     if (!ReleaseMutex(mutex)) {
-        return -1;
+        // TODO(yuriks): Verify error code, this one was pulled out of thin air. I'm not even sure
+        // what error condition this is supposed to be signaling.
+        return ResultCode(ErrorDescription::AlreadyDone, ErrorModule::Kernel,
+                ErrorSummary::NothingHappened, ErrorLevel::Temporary);
     }
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**

src/core/hle/kernel/mutex.h

@@ -15,7 +15,7 @@ namespace Kernel {
  * @param handle Handle to mutex to release
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result ReleaseMutex(Handle handle);
+ResultCode ReleaseMutex(Handle handle);
 
 /**
  * Creates a mutex

src/core/hle/kernel/shared_memory.cpp

@@ -21,10 +21,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)");
-        return 0;
+        return UnimplementedFunction(ErrorModule::OS);
     }
 
     u32 base_address;                   ///< Address of shared memory block in RAM
@@ -67,22 +67,23 @@ Handle CreateSharedMemory(const std::string& name) {
  * @param other_permissions Memory block map other permissions (specified by SVC field)
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions,
+ResultCode MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions,
     MemoryPermission other_permissions) {
 
     if (address < Memory::SHARED_MEMORY_VADDR || address >= Memory::SHARED_MEMORY_VADDR_END) {
         ERROR_LOG(KERNEL, "cannot map handle=0x%08X, address=0x%08X outside of shared mem bounds!",
             handle, address);
-        return -1;
+        return ResultCode(ErrorDescription::InvalidAddress, ErrorModule::Kernel,
+                ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
     }
-    SharedMemory* shared_memory = Kernel::g_object_pool.GetFast<SharedMemory>(handle);
+    SharedMemory* shared_memory = Kernel::g_object_pool.Get<SharedMemory>(handle);
-    _assert_msg_(KERNEL, (shared_memory != nullptr), "handle 0x%08X is not valid!", handle);
+    if (shared_memory == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     shared_memory->base_address = address;
     shared_memory->permissions = permissions;
     shared_memory->other_permissions = other_permissions;
 
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /**
@@ -91,15 +92,17 @@ Result MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions,
  * @param offset Offset from the start of the shared memory block to get pointer
  * @return Pointer to the shared memory block from the specified offset
  */
-u8* GetSharedMemoryPointer(Handle handle, u32 offset) {
+ResultVal<u8*> GetSharedMemoryPointer(Handle handle, u32 offset) {
-    SharedMemory* shared_memory = Kernel::g_object_pool.GetFast<SharedMemory>(handle);
+    SharedMemory* shared_memory = Kernel::g_object_pool.Get<SharedMemory>(handle);
-    _assert_msg_(KERNEL, (shared_memory != nullptr), "handle 0x%08X is not valid!", handle);
+    if (shared_memory == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     if (0 != shared_memory->base_address)
-        return Memory::GetPointer(shared_memory->base_address + offset);
+        return MakeResult<u8*>(Memory::GetPointer(shared_memory->base_address + offset));
 
     ERROR_LOG(KERNEL, "memory block handle=0x%08X not mapped!", handle);
-    return nullptr;
+    // TODO(yuriks): Verify error code.
+    return ResultCode(ErrorDescription::InvalidAddress, ErrorModule::Kernel,
+            ErrorSummary::InvalidState, ErrorLevel::Permanent);
 }
 
 } // namespace

src/core/hle/kernel/shared_memory.h

@@ -34,7 +34,7 @@ Handle CreateSharedMemory(const std::string& name="Unknown");
  * @param other_permissions Memory block map other permissions (specified by SVC field)
  * @return Result of operation, 0 on success, otherwise error code
  */
-Result MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions,
+ResultCode MapSharedMemory(Handle handle, u32 address, MemoryPermission permissions,
     MemoryPermission other_permissions);
 
 /**
@@ -43,6 +43,6 @@ Result MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions,
  * @param offset Offset from the start of the shared memory block to get pointer
  * @return Pointer to the shared memory block from the specified offset
  */
-u8* GetSharedMemoryPointer(Handle handle, u32 offset);
+ResultVal<u8*> GetSharedMemoryPointer(Handle handle, u32 offset);
 
 } // namespace

src/core/hle/kernel/thread.cpp

@@ -11,10 +11,11 @@
 #include "common/thread_queue_list.h"
 
 #include "core/core.h"
-#include "core/mem_map.h"
 #include "core/hle/hle.h"
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/kernel/thread.h"
+#include "core/hle/result.h"
+#include "core/mem_map.h"
 
 namespace Kernel {
 
@@ -38,16 +39,17 @@ 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 {
         if (status != THREADSTATUS_DORMANT) {
             Handle thread = GetCurrentThreadHandle();
             if (std::find(waiting_threads.begin(), waiting_threads.end(), thread) == waiting_threads.end()) {
                 waiting_threads.push_back(thread);
             }
             WaitCurrentThread(WAITTYPE_THREADEND, this->GetHandle());
-            *wait = true;
+            return MakeResult<bool>(true);
+        } else {
+            return MakeResult<bool>(false);
         }
-        return 0;
     }
 
     ThreadContext context;
@@ -144,9 +146,9 @@ void ChangeReadyState(Thread* t, bool ready) {
 }
 
 /// Verify that a thread has not been released from waiting
-inline bool VerifyWait(const Handle& handle, WaitType type, Handle wait_handle) {
+inline bool VerifyWait(Handle handle, WaitType type, Handle wait_handle) {
-    Thread* thread = g_object_pool.GetFast<Thread>(handle);
+    Thread* thread = g_object_pool.Get<Thread>(handle);
-    _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!");
+    _dbg_assert_(KERNEL, thread != nullptr);
 
     if (type != thread->wait_type || wait_handle != thread->wait_handle)
         return false;
@@ -155,9 +157,9 @@ inline bool VerifyWait(const Handle& handle, WaitType type, Handle wait_handle)
 }
 
 /// Stops the current thread
-void StopThread(Handle handle, const char* reason) {
+ResultCode StopThread(Handle handle, const char* reason) {
-    Thread* thread = g_object_pool.GetFast<Thread>(handle);
+    Thread* thread = g_object_pool.Get<Thread>(handle);
-    _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!");
+    if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel);
 
     ChangeReadyState(thread, false);
     thread->status = THREADSTATUS_DORMANT;
@@ -172,6 +174,8 @@ void StopThread(Handle handle, const char* reason) {
     // Stopped threads are never waiting.
     thread->wait_type = WAITTYPE_NONE;
     thread->wait_handle = 0;
+
+    return RESULT_SUCCESS;
 }
 
 /// Changes a threads state
@@ -201,7 +205,9 @@ Handle ArbitrateHighestPriorityThread(u32 arbiter, u32 address) {
         if (!VerifyWait(handle, WAITTYPE_ARB, arbiter))
             continue;
 
-        Thread* thread = g_object_pool.GetFast<Thread>(handle);
+        Thread* thread = g_object_pool.Get<Thread>(handle);
+        if (thread == nullptr)
+            continue; // TODO(yuriks): Thread handle will hang around forever. Should clean up.
         if(thread->current_priority <= priority) {
             highest_priority_thread = handle;
             priority = thread->current_priority;
@@ -272,7 +278,7 @@ Thread* NextThread() {
     if (next == 0) {
         return nullptr;
     }
-    return Kernel::g_object_pool.GetFast<Thread>(next);
+    return Kernel::g_object_pool.Get<Thread>(next);
 }
 
 /**
@@ -289,8 +295,7 @@ void WaitCurrentThread(WaitType wait_type, Handle wait_handle) {
 
 /// Resumes a thread from waiting by marking it as "ready"
 void ResumeThreadFromWait(Handle handle) {
-    u32 error;
+    Thread* thread = Kernel::g_object_pool.Get<Thread>(handle);
-    Thread* thread = Kernel::g_object_pool.Get<Thread>(handle, error);
     if (thread) {
         thread->status &= ~THREADSTATUS_WAIT;
         if (!(thread->status & (THREADSTATUS_WAITSUSPEND | THREADSTATUS_DORMANT | THREADSTATUS_DEAD))) {
@@ -378,19 +383,23 @@ Handle CreateThread(const char* name, u32 entry_point, s32 priority, u32 arg, s3
 }
 
 /// Get the priority of the thread specified by handle
-u32 GetThreadPriority(const Handle handle) {
+ResultVal<u32> GetThreadPriority(const Handle handle) {
-    Thread* thread = g_object_pool.GetFast<Thread>(handle);
+    Thread* thread = g_object_pool.Get<Thread>(handle);
-    _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!");
+    if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel);
-    return thread->current_priority;
+
+    return MakeResult<u32>(thread->current_priority);
 }
 
 /// Set the priority of the thread specified by handle
-Result SetThreadPriority(Handle handle, s32 priority) {
+ResultCode SetThreadPriority(Handle handle, s32 priority) {
     Thread* thread = nullptr;
     if (!handle) {
         thread = GetCurrentThread(); // TODO(bunnei): Is this correct behavior?
     } else {
-        thread = g_object_pool.GetFast<Thread>(handle);
+        thread = g_object_pool.Get<Thread>(handle);
+        if (thread == nullptr) {
+            return InvalidHandle(ErrorModule::Kernel);
+        }
     }
     _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!");
 
@@ -417,7 +426,7 @@ Result SetThreadPriority(Handle handle, s32 priority) {
         thread_ready_queue.push_back(thread->current_priority, handle);
     }
 
-    return 0;
+    return RESULT_SUCCESS;
 }
 
 /// Sets up the primary application thread

src/core/hle/kernel/thread.h

@@ -6,6 +6,7 @@
 
 #include "common/common_types.h"
 #include "core/hle/kernel/kernel.h"
+#include "core/hle/result.h"
 
 enum ThreadPriority {
     THREADPRIO_HIGHEST      = 0,    ///< Highest thread priority
@@ -55,7 +56,7 @@ Handle SetupMainThread(s32 priority, int stack_size=Kernel::DEFAULT_STACK_SIZE);
 void Reschedule();
 
 /// Stops the current thread
-void StopThread(Handle thread, const char* reason);
+ResultCode StopThread(Handle thread, const char* reason);
 
 /// Resumes a thread from waiting by marking it as "ready"
 void ResumeThreadFromWait(Handle handle);
@@ -80,10 +81,10 @@ void WaitCurrentThread(WaitType wait_type, Handle wait_handle=GetCurrentThreadHa
 void WaitThread_Synchronization();
 
 /// Get the priority of the thread specified by handle
-u32 GetThreadPriority(const Handle handle);
+ResultVal<u32> GetThreadPriority(const Handle handle);
 
 /// Set the priority of the thread specified by handle
-Result SetThreadPriority(Handle handle, s32 priority);
+ResultCode SetThreadPriority(Handle handle, s32 priority);
 
 /// Initialize threading
 void ThreadingInit();

src/core/hle/result.h

@@ -0,0 +1,400 @@
+// Copyright 2014 Citra Emulator Project
+// Licensed under GPLv2
+// Refer to the license.txt file included.
+
+#pragma once
+
+#include <cassert>
+#include <cstddef>
+#include <type_traits>
+#include <utility>
+
+#include "common/common_types.h"
+#include "common/bit_field.h"
+
+// All the constants in this file come from http://3dbrew.org/wiki/Error_codes
+
+/// Detailed description of the error. This listing is likely incomplete.
+enum class ErrorDescription : u32 {
+    Success = 0,
+    InvalidSection = 1000,
+    TooLarge = 1001,
+    NotAuthorized = 1002,
+    AlreadyDone = 1003,
+    InvalidSize = 1004,
+    InvalidEnumValue = 1005,
+    InvalidCombination = 1006,
+    NoData = 1007,
+    Busy = 1008,
+    MisalignedAddress = 1009,
+    MisalignedSize = 1010,
+    OutOfMemory = 1011,
+    NotImplemented = 1012,
+    InvalidAddress = 1013,
+    InvalidPointer = 1014,
+    InvalidHandle = 1015,
+    NotInitialized = 1016,
+    AlreadyInitialized = 1017,
+    NotFound = 1018,
+    CancelRequested = 1019,
+    AlreadyExists = 1020,
+    OutOfRange = 1021,
+    Timeout = 1022,
+    InvalidResultValue = 1023,
+};
+
+/**
+ * Identifies the module which caused the error. Error codes can be propagated through a call
+ * chain, meaning that this doesn't always correspond to the module where the API call made is
+ * contained.
+ */
+enum class ErrorModule : u32 {
+    Common = 0,
+    Kernel = 1,
+    Util = 2,
+    FileServer = 3,
+    LoaderServer = 4,
+    TCB = 5,
+    OS = 6,
+    DBG = 7,
+    DMNT = 8,
+    PDN = 9,
+    GX = 10,
+    I2C = 11,
+    GPIO = 12,
+    DD = 13,
+    CODEC = 14,
+    SPI = 15,
+    PXI = 16,
+    FS = 17,
+    DI = 18,
+    HID = 19,
+    CAM = 20,
+    PI = 21,
+    PM = 22,
+    PM_LOW = 23,
+    FSI = 24,
+    SRV = 25,
+    NDM = 26,
+    NWM = 27,
+    SOC = 28,
+    LDR = 29,
+    ACC = 30,
+    RomFS = 31,
+    AM = 32,
+    HIO = 33,
+    Updater = 34,
+    MIC = 35,
+    FND = 36,
+    MP = 37,
+    MPWL = 38,
+    AC = 39,
+    HTTP = 40,
+    DSP = 41,
+    SND = 42,
+    DLP = 43,
+    HIO_LOW = 44,
+    CSND = 45,
+    SSL = 46,
+    AM_LOW = 47,
+    NEX = 48,
+    Friends = 49,
+    RDT = 50,
+    Applet = 51,
+    NIM = 52,
+    PTM = 53,
+    MIDI = 54,
+    MC = 55,
+    SWC = 56,
+    FatFS = 57,
+    NGC = 58,
+    CARD = 59,
+    CARDNOR = 60,
+    SDMC = 61,
+    BOSS = 62,
+    DBM = 63,
+    Config = 64,
+    PS = 65,
+    CEC = 66,
+    IR = 67,
+    UDS = 68,
+    PL = 69,
+    CUP = 70,
+    Gyroscope = 71,
+    MCU = 72,
+    NS = 73,
+    News = 74,
+    RO_1 = 75,
+    GD = 76,
+    CardSPI = 77,
+    EC = 78,
+    RO_2 = 79,
+    WebBrowser = 80,
+    Test = 81,
+    ENC = 82,
+    PIA = 83,
+
+    Application = 254,
+    InvalidResult = 255
+};
+
+/// A less specific error cause.
+enum class ErrorSummary : u32 {
+    Success = 0,
+    NothingHappened = 1,
+    WouldBlock = 2,
+    OutOfResource = 3,      ///< There are no more kernel resources (memory, table slots) to
+                            ///< execute the operation.
+    NotFound = 4,           ///< A file or resource was not found.
+    InvalidState = 5,
+    NotSupported = 6,       ///< The operation is not supported or not implemented.
+    InvalidArgument = 7,    ///< Returned when a passed argument is invalid in the current runtime
+                            ///< context. (Invalid handle, out-of-bounds pointer or size, etc.)
+    WrongArgument = 8,      ///< Returned when a passed argument is in an incorrect format for use
+                            ///< with the function. (E.g. Invalid enum value)
+    Canceled = 9,
+    StatusChanged = 10,
+    Internal = 11,
+
+    InvalidResult = 63
+};
+
+/// The severity of the error.
+enum class ErrorLevel : u32 {
+    Success = 0,
+    Info = 1,
+
+    Status = 25,
+    Temporary = 26,
+    Permanent = 27,
+    Usage = 28,
+    Reinitialize = 29,
+    Reset = 30,
+    Fatal = 31
+};
+
+/// Encapsulates a CTR-OS error code, allowing it to be separated into its constituent fields.
+union ResultCode {
+    u32 raw;
+
+    BitField<0, 10, ErrorDescription> description;
+    BitField<10, 8, ErrorModule> module;
+
+    BitField<21, 6, ErrorSummary> summary;
+    BitField<27, 5, ErrorLevel> level;
+
+    // The last bit of `level` is checked by apps and the kernel to determine if a result code is an error
+    BitField<31, 1, u32> is_error;
+
+    explicit ResultCode(u32 raw) : raw(raw) {}
+    ResultCode(ErrorDescription description_, ErrorModule module_,
+            ErrorSummary summary_, ErrorLevel level_) : raw(0) {
+        description = description_;
+        module = module_;
+        summary = summary_;
+        level = level_;
+    }
+
+    ResultCode& operator=(const ResultCode& o) { raw = o.raw; return *this; }
+
+    bool IsSuccess() const {
+        return is_error == 0;
+    }
+
+    bool IsError() const {
+        return is_error == 1;
+    }
+};
+
+inline bool operator==(const ResultCode a, const ResultCode b) {
+    return a.raw == b.raw;
+}
+
+inline bool operator!=(const ResultCode a, const ResultCode b) {
+    return a.raw != b.raw;
+}
+
+// Convenience functions for creating some common kinds of errors:
+
+/// The default success `ResultCode`.
+const ResultCode RESULT_SUCCESS(0);
+
+/// Might be returned instead of a dummy success for unimplemented APIs.
+inline ResultCode UnimplementedFunction(ErrorModule module) {
+    return ResultCode(ErrorDescription::NotImplemented, module,
+            ErrorSummary::NotSupported, ErrorLevel::Permanent);
+}
+/// Returned when a function is passed an invalid handle.
+inline ResultCode InvalidHandle(ErrorModule module) {
+    return ResultCode(ErrorDescription::InvalidHandle, module,
+            ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
+}
+
+/**
+ * This is an optional value type. It holds a `ResultCode` and, if that code is a success code,
+ * also holds a result of type `T`. If the code is an error code then trying to access the inner
+ * value fails, thus ensuring that the ResultCode of functions is always checked properly before
+ * their return value is used.  It is similar in concept to the `std::optional` type
+ * (http://en.cppreference.com/w/cpp/experimental/optional) originally proposed for inclusion in
+ * C++14, or the `Result` type in Rust (http://doc.rust-lang.org/std/result/index.html).
+ *
+ * An example of how it could be used:
+ * \code
+ * ResultVal<int> Frobnicate(float strength) {
+ *     if (strength < 0.f || strength > 1.0f) {
+ *         // Can't frobnicate too weakly or too strongly
+ *         return ResultCode(ErrorDescription::OutOfRange, ErrorModule::Common,
+ *             ErrorSummary::InvalidArgument, ErrorLevel::Permanent);
+ *     } else {
+ *         // Frobnicated! Give caller a cookie
+ *         return MakeResult<int>(42);
+ *     }
+ * }
+ * \endcode
+ *
+ * \code
+ * ResultVal<int> frob_result = Frobnicate(0.75f);
+ * if (frob_result) {
+ *     // Frobbed ok
+ *     printf("My cookie is %d\n", *frob_result);
+ * } else {
+ *     printf("Guess I overdid it. :( Error code: %ux\n", frob_result.code().hex);
+ * }
+ * \endcode
+ */
+template <typename T>
+class ResultVal {
+public:
+    /// Constructs an empty `ResultVal` with the given error code. The code must not be a success code.
+    ResultVal(ResultCode error_code = ResultCode(-1))
+        : result_code(error_code)
+    {
+        assert(error_code.IsError());
+        UpdateDebugPtr();
+    }
+
+    /**
+     * Similar to the non-member function `MakeResult`, with the exception that you can manually
+     * specify the success code. `success_code` must not be an error code.
+     */
+    template <typename... Args>
+    static ResultVal WithCode(ResultCode success_code, Args&&... args) {
+        ResultVal<T> result;
+        result.emplace(success_code, std::forward<Args>(args)...);
+        return result;
+    }
+
+    ResultVal(const ResultVal& o)
+        : result_code(o.result_code)
+    {
+        if (!o.empty()) {
+            new (&storage) T(*o.GetPointer());
+        }
+        UpdateDebugPtr();
+    }
+
+    ResultVal(ResultVal&& o)
+        : result_code(o.result_code)
+    {
+        if (!o.empty()) {
+            new (&storage) T(std::move(*o.GetPointer()));
+        }
+        UpdateDebugPtr();
+    }
+
+    ~ResultVal() {
+        if (!empty()) {
+            GetPointer()->~T();
+        }
+    }
+
+    ResultVal& operator=(const ResultVal& o) {
+        if (*this) {
+            if (o) {
+                *GetPointer() = *o.GetPointer();
+            } else {
+                GetPointer()->~T();
+            }
+        } else {
+            if (o) {
+                new (&storage) T(*o.GetPointer());
+            }
+        }
+        result_code = o.result_code;
+        UpdateDebugPtr();
+
+        return *this;
+    }
+
+    /**
+     * Replaces the current result with a new constructed result value in-place. The code must not
+     * be an error code.
+     */
+    template <typename... Args>
+    void emplace(ResultCode success_code, Args&&... args) {
+        assert(success_code.IsSuccess());
+        if (!empty()) {
+            GetPointer()->~T();
+        }
+        new (&storage) T(std::forward<Args>(args)...);
+        result_code = success_code;
+        UpdateDebugPtr();
+    }
+
+    /// Returns true if the `ResultVal` contains an error code and no value.
+    bool empty() const { return result_code.IsError(); }
+
+    /// Returns true if the `ResultVal` contains a return value.
+    bool Succeeded() const { return result_code.IsSuccess(); }
+    /// Returns true if the `ResultVal` contains an error code and no value.
+    bool Failed() const { return empty(); }
+
+    ResultCode Code() const { return result_code; }
+
+    const T& operator* () const { return *GetPointer(); }
+          T& operator* ()       { return *GetPointer(); }
+    const T* operator->() const { return  GetPointer(); }
+          T* operator->()       { return  GetPointer(); }
+
+    /// Returns the value contained in this `ResultVal`, or the supplied default if it is missing.
+    template <typename U>
+    T ValueOr(U&& value) const {
+        return !empty() ? *GetPointer() : std::move(value);
+    }
+
+private:
+    typedef typename std::aligned_storage<sizeof(T), std::alignment_of<T>::value>::type StorageType;
+
+    StorageType storage;
+    ResultCode result_code;
+#if _DEBUG
+    // The purpose of this pointer is to aid inspecting the type with a debugger, eliminating the
+    // need to cast `storage` to a pointer or pay attention to `result_code`.
+    const T* debug_ptr;
+#endif
+
+    void UpdateDebugPtr() {
+#if _DEBUG
+        debug_ptr = empty() ? nullptr : static_cast<const T*>(static_cast<const void*>(&storage));
+#endif
+    }
+
+    const T* GetPointer() const {
+        assert(!empty());
+        return static_cast<const T*>(static_cast<const void*>(&storage));
+    }
+
+    T* GetPointer() {
+        assert(!empty());
+        return static_cast<T*>(static_cast<void*>(&storage));
+    }
+};
+
+/**
+ * This function is a helper used to construct `ResultVal`s. It receives the arguments to construct
+ * `T` with and creates a success `ResultVal` contained the constructed value.
+ */
+template <typename T, typename... Args>
+ResultVal<T> MakeResult(Args&&... args) {
+    return ResultVal<T>::WithCode(RESULT_SUCCESS, std::forward<Args>(args)...);
+}

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);
+    ResultVal<Handle> handle = Kernel::OpenFileFromArchive(archive_handle, file_path, mode);
-    if (handle) {
+    cmd_buff[1] = handle.Code().raw;
-        cmd_buff[1] = 0;
+    if (handle.Succeeded()) {
-        cmd_buff[3] = handle;
+        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);
+    // TODO(yuriks): Why is there all this duplicate (and seemingly useless) code up here?
-    if (!archive_handle) {
+    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);
+    ResultVal<Handle> handle = Kernel::OpenFileFromArchive(*archive_handle, file_path, mode);
-    if (handle) {
+    cmd_buff[1] = handle.Code().raw;
-        cmd_buff[1] = 0;
+    if (handle.Succeeded()) {
-        cmd_buff[3] = handle;
+        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);
+    ResultVal<Handle> handle = Kernel::OpenDirectoryFromArchive(archive_handle, dir_path);
-    if (handle) {
+    cmd_buff[1] = handle.Code().raw;
-        cmd_buff[1] = 0;
+    if (handle.Succeeded()) {
-        cmd_buff[3] = handle;
+        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);
+    ResultVal<Handle> handle = Kernel::OpenArchive(archive_id);
-    if (handle) {
+    cmd_buff[1] = handle.Code().raw;
-        cmd_buff[1] = 0;
+    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");

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)
+    ResultVal<u8*> ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, 0x800 + (thread_id * sizeof(CommandBuffer)));
-        return nullptr;
+    return ptr.ValueOr(nullptr);
-
-    return Kernel::GetSharedMemoryPointer(g_shared_memory,
-        0x800 + (thread_id * sizeof(CommandBuffer)));
 }
 
 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,
+    ResultVal<u8*> ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, sizeof(InterruptRelayQueue) * thread_id);
-        sizeof(InterruptRelayQueue) * thread_id);
+    return reinterpret_cast<InterruptRelayQueue*>(ptr.ValueOr(nullptr));
 }
 
 static void WriteHWRegs(u32 base_address, u32 size_in_bytes, const u32* data) {

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 reinterpret_cast<PadData*>(Kernel::GetSharedMemoryPointer(shared_mem, 0).ValueOr(nullptr));
-        return nullptr;
-
-    return reinterpret_cast<PadData*>(Kernel::GetSharedMemoryPointer(shared_mem, 0));
 }
 
 /**

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

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:

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[] = {

src/core/hle/svc.cpp

@@ -16,6 +16,7 @@
 #include "core/hle/kernel/thread.h"
 
 #include "core/hle/function_wrappers.h"
+#include "core/hle/result.h"
 #include "core/hle/service/service.h"
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -86,18 +87,22 @@ static Result ConnectToPort(Handle* out, const char* port_name) {
 
 /// Synchronize to an OS service
 static Result SendSyncRequest(Handle handle) {
+    // TODO(yuriks): ObjectPool::Get tries to check the Object type, which fails since this is a generic base Object,
+    // so we are forced to use GetFast and manually verify the handle.
+    if (!Kernel::g_object_pool.IsValid(handle)) {
+        return InvalidHandle(ErrorModule::Kernel).raw;
+    }
     Kernel::Object* object = Kernel::g_object_pool.GetFast<Kernel::Object>(handle);
 
     _assert_msg_(KERNEL, (object != nullptr), "called, but kernel object is nullptr!");
     DEBUG_LOG(SVC, "called handle=0x%08X(%s)", handle, object->GetTypeName().c_str());
 
-    bool wait = false;
+    ResultVal<bool> wait = object->SyncRequest();
-    Result res = object->SyncRequest(&wait);
+    if (wait.Succeeded() && *wait) {
-    if (wait) {
         Kernel::WaitCurrentThread(WAITTYPE_SYNCH); // TODO(bunnei): Is this correct?
     }
 
-    return res;
+    return wait.Code().raw;
 }
 
 /// Close a handle
@@ -110,25 +115,25 @@ static Result CloseHandle(Handle handle) {
 /// Wait for a handle to synchronize, timeout after the specified nanoseconds
 static Result WaitSynchronization1(Handle handle, s64 nano_seconds) {
     // TODO(bunnei): Do something with nano_seconds, currently ignoring this
-    bool wait = false;
     bool wait_infinite = (nano_seconds == -1); // Used to wait until a thread has terminated
 
+    if (!Kernel::g_object_pool.IsValid(handle)) {
+        return InvalidHandle(ErrorModule::Kernel).raw;
+    }
     Kernel::Object* object = Kernel::g_object_pool.GetFast<Kernel::Object>(handle);
+    _dbg_assert_(KERNEL, object != nullptr);
 
     DEBUG_LOG(SVC, "called handle=0x%08X(%s:%s), nanoseconds=%lld", handle, object->GetTypeName().c_str(),
             object->GetName().c_str(), nano_seconds);
 
-    _assert_msg_(KERNEL, (object != nullptr), "called, but kernel object is nullptr!");
+    ResultVal<bool> wait = object->WaitSynchronization();
-
-    Result res = object->WaitSynchronization(&wait);
 
     // Check for next thread to schedule
-    if (wait) {
+    if (wait.Succeeded() && *wait) {
         HLE::Reschedule(__func__);
-        return 0;
     }
 
-    return res;
+    return wait.Code().raw;
 }
 
 /// Wait for the given handles to synchronize, timeout after the specified nanoseconds
@@ -143,20 +148,21 @@ static Result WaitSynchronizationN(s32* out, Handle* handles, s32 handle_count,
 
     // Iterate through each handle, synchronize kernel object
     for (s32 i = 0; i < handle_count; i++) {
-        bool wait = false;
+        if (!Kernel::g_object_pool.IsValid(handles[i])) {
+            return InvalidHandle(ErrorModule::Kernel).raw;
+        }
         Kernel::Object* object = Kernel::g_object_pool.GetFast<Kernel::Object>(handles[i]);
 
-        _assert_msg_(KERNEL, (object != nullptr), "called handle=0x%08X, but kernel object "
-            "is nullptr!", handles[i]);
-
         DEBUG_LOG(SVC, "\thandle[%d] = 0x%08X(%s:%s)", i, handles[i], object->GetTypeName().c_str(),
             object->GetName().c_str());
 
-        Result res = object->WaitSynchronization(&wait);
+        // TODO(yuriks): Verify how the real function behaves when an error happens here
+        ResultVal<bool> wait_result = object->WaitSynchronization();
+        bool wait = wait_result.Succeeded() && *wait_result;
 
         if (!wait && !wait_all) {
             *out = i;
-            return 0;
+            return RESULT_SUCCESS.raw;
         } else {
             unlock_all = false;
         }
@@ -164,13 +170,13 @@ static Result WaitSynchronizationN(s32* out, Handle* handles, s32 handle_count,
 
     if (wait_all && unlock_all) {
         *out = handle_count;
-        return 0;
+        return RESULT_SUCCESS.raw;
     }
 
     // Check for next thread to schedule
     HLE::Reschedule(__func__);
 
-    return 0;
+    return RESULT_SUCCESS.raw;
 }
 
 /// Create an address arbiter (to allocate access to shared resources)
@@ -183,8 +189,8 @@ static Result CreateAddressArbiter(u32* arbiter) {
 
 /// Arbitrate address
 static Result ArbitrateAddress(Handle arbiter, u32 address, u32 type, u32 value, s64 nanoseconds) {
-    return Kernel::ArbitrateAddress(arbiter, static_cast<Kernel::ArbitrationType>(type), address,
+    return Kernel::ArbitrateAddress(arbiter, static_cast<Kernel::ArbitrationType>(type),
-        value);
+            address, value).raw;
 }
 
 /// Used to output a message on a debug hardware unit - does nothing on a retail unit
@@ -246,13 +252,16 @@ static u32 ExitThread() {
 
 /// Gets the priority for the specified thread
 static Result GetThreadPriority(s32* priority, Handle handle) {
-    *priority = Kernel::GetThreadPriority(handle);
+    ResultVal<u32> priority_result = Kernel::GetThreadPriority(handle);
-    return 0;
+    if (priority_result.Succeeded()) {
+        *priority = *priority_result;
+    }
+    return priority_result.Code().raw;
 }
 
 /// Sets the priority for the specified thread
 static Result SetThreadPriority(Handle handle, s32 priority) {
-    return Kernel::SetThreadPriority(handle, priority);
+    return Kernel::SetThreadPriority(handle, priority).raw;
 }
 
 /// Create a mutex
@@ -266,9 +275,8 @@ static Result CreateMutex(Handle* mutex, u32 initial_locked) {
 /// Release a mutex
 static Result ReleaseMutex(Handle handle) {
     DEBUG_LOG(SVC, "called handle=0x%08X", handle);
-    _assert_msg_(KERNEL, (handle != 0), "called, but handle is nullptr!");
+    ResultCode res = Kernel::ReleaseMutex(handle);
-    Kernel::ReleaseMutex(handle);
+    return res.raw;
-    return 0;
 }
 
 /// Get current thread ID
@@ -310,16 +318,14 @@ static Result DuplicateHandle(Handle* out, Handle handle) {
 
 /// Signals an event
 static Result SignalEvent(Handle evt) {
-    Result res = Kernel::SignalEvent(evt);
     DEBUG_LOG(SVC, "called event=0x%08X", evt);
-    return res;
+    return Kernel::SignalEvent(evt).raw;
 }
 
 /// Clears an event
 static Result ClearEvent(Handle evt) {
-    Result res = Kernel::ClearEvent(evt);
     DEBUG_LOG(SVC, "called event=0x%08X", evt);
-    return res;
+    return Kernel::ClearEvent(evt).raw;
 }
 
 /// Sleep the current thread