From 924bbde89b80ff2aa8b98fe8f3c7f728ede34edc Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sat, 25 Oct 2014 17:52:39 -0200 Subject: [PATCH 1/4] Change some SkyEye defines to const ints This prevents them from interfering with other constants defined in different namespaces. --- src/core/arm/skyeye_common/armdefs.h | 32 ++++++++++++++-------------- src/core/arm/skyeye_common/armemu.h | 18 ---------------- 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/core/arm/skyeye_common/armdefs.h b/src/core/arm/skyeye_common/armdefs.h index 8e71948c6..6e8187c8e 100644 --- a/src/core/arm/skyeye_common/armdefs.h +++ b/src/core/arm/skyeye_common/armdefs.h @@ -799,22 +799,22 @@ pascal void SpinCursor (short increment); /* copied from CursorCtl.h */ #include "list.h" #include "tb.h" */ -#define EQ 0 -#define NE 1 -#define CS 2 -#define CC 3 -#define MI 4 -#define PL 5 -#define VS 6 -#define VC 7 -#define HI 8 -#define LS 9 -#define GE 10 -#define LT 11 -#define GT 12 -#define LE 13 -#define AL 14 -#define NV 15 +const int EQ = 0; +const int NE = 1; +const int CS = 2; +const int CC = 3; +const int MI = 4; +const int PL = 5; +const int VS = 6; +const int VC = 7; +const int HI = 8; +const int LS = 9; +const int GE = 10; +const int LT = 11; +const int GT = 12; +const int LE = 13; +const int AL = 14; +const int NV = 15; #ifndef NFLAG #define NFLAG state->NFlag diff --git a/src/core/arm/skyeye_common/armemu.h b/src/core/arm/skyeye_common/armemu.h index c0f0270fe..075fc7e9e 100644 --- a/src/core/arm/skyeye_common/armemu.h +++ b/src/core/arm/skyeye_common/armemu.h @@ -25,24 +25,6 @@ #define DEBUG(...) DEBUG_LOG(ARM11, __VA_ARGS__) -/* Condition code values. */ -#define EQ 0 -#define NE 1 -#define CS 2 -#define CC 3 -#define MI 4 -#define PL 5 -#define VS 6 -#define VC 7 -#define HI 8 -#define LS 9 -#define GE 10 -#define LT 11 -#define GT 12 -#define LE 13 -#define AL 14 -#define NV 15 - /* Shift Opcodes. */ #define LSL 0 #define LSR 1 From c2588403c0b8cf198f13f903f626851c7e94266c Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 23 Oct 2014 01:20:01 -0200 Subject: [PATCH 2/4] 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 | 1 + src/core/arm/skyeye_common/armdefs.h | 34 +- src/core/hle/kernel/address_arbiter.cpp | 10 +- src/core/hle/kernel/address_arbiter.h | 2 +- src/core/hle/kernel/archive.cpp | 147 ++++----- src/core/hle/kernel/archive.h | 23 +- src/core/hle/kernel/event.cpp | 38 +-- src/core/hle/kernel/event.h | 8 +- src/core/hle/kernel/kernel.h | 44 +-- src/core/hle/kernel/mutex.cpp | 25 +- src/core/hle/kernel/mutex.h | 2 +- src/core/hle/kernel/shared_memory.cpp | 27 +- src/core/hle/kernel/shared_memory.h | 4 +- src/core/hle/kernel/thread.cpp | 51 +-- src/core/hle/kernel/thread.h | 7 +- src/core/hle/result.h | 400 ++++++++++++++++++++++++ src/core/hle/service/fs_user.cpp | 66 ++-- src/core/hle/service/gsp_gpu.cpp | 17 +- src/core/hle/service/hid_user.cpp | 5 +- src/core/hle/service/service.cpp | 2 +- src/core/hle/service/service.h | 12 +- src/core/hle/service/srv.cpp | 6 +- src/core/hle/svc.cpp | 68 ++-- 23 files changed, 689 insertions(+), 310 deletions(-) create mode 100644 src/core/hle/result.h diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index b3bfbca9e..48241c3d4 100644 --- a/src/core/CMakeLists.txt +++ b/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 diff --git a/src/core/arm/skyeye_common/armdefs.h b/src/core/arm/skyeye_common/armdefs.h index 6e8187c8e..8343aaa01 100644 --- a/src/core/arm/skyeye_common/armdefs.h +++ b/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; -const int NE = 1; -const int CS = 2; -const int CC = 3; -const int MI = 4; -const int PL = 5; -const int VS = 6; -const int VC = 7; -const int HI = 8; -const int LS = 9; -const int GE = 10; -const int LT = 11; -const int GT = 12; -const int LE = 13; -const int AL = 14; -const int NV = 15; +enum ConditionCode { + EQ = 0, + NE = 1, + CS = 2, + CC = 3, + MI = 4, + PL = 5, + VS = 6, + VC = 7, + HI = 8, + LS = 9, + GE = 10, + LT = 11, + GT = 12, + LE = 13, + AL = 14, + NV = 15, +}; #ifndef NFLAG #define NFLAG state->NFlag diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp index 2b21657da..1e697fac1 100644 --- a/src/core/hle/kernel/address_arbiter.cpp +++ b/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 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 diff --git a/src/core/hle/kernel/address_arbiter.h b/src/core/hle/kernel/address_arbiter.h index 6886e479d..8a5fb10b4 100644 --- a/src/core/hle/kernel/address_arbiter.h +++ b/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"); diff --git a/src/core/hle/kernel/archive.cpp b/src/core/hle/kernel/archive.cpp index 900f484c7..e11dddc84 100644 --- a/src/core/hle/kernel/archive.cpp +++ b/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 SyncRequest() override { u32* cmd_buff = Service::GetCommandBuffer(); FileCommand cmd = static_cast(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(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 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 SyncRequest() override { u32* cmd_buff = Service::GetCommandBuffer(); FileCommand cmd = static_cast(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. - return -1; + ResultCode error = UnimplementedFunction(ErrorModule::FS); + 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(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 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 SyncRequest() override { u32* cmd_buff = Service::GetCommandBuffer(); DirectoryCommand cmd = static_cast(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(Memory::GetPointer(address)); - DEBUG_LOG(KERNEL, "Read %s %s: count=%d", GetTypeName().c_str(), GetName().c_str(), count); + auto entries = reinterpret_cast(Memory::GetPointer(address)); + 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. - return -1; + ResultCode error = UnimplementedFunction(ErrorModule::FS); + 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(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 WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "(UNIMPLEMENTED)"); - return 0; + return UnimplementedFunction(ErrorModule::FS); } }; @@ -284,89 +289,59 @@ public: std::map g_archive_map; ///< Map of file archives by IdCode -/** - * 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) { +ResultVal 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(itr->second); } -/** - * Closes an archive - * @param id_code IdCode of the archive to open - * @return Result of operation, 0 on success, otherwise error code - */ -Result CloseArchive(FileSys::Archive::IdCode id_code) { +ResultCode 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 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; } -/** - * 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) { +ResultCode CreateArchive(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); - - return archive; + ResultCode result = MountArchive(archive); + if (result.IsError()) { + 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; -} - -/** - * 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) { +ResultVal 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(archive_handle); File* file = new File; Handle handle = Kernel::g_object_pool.Create(file); - Archive* archive = Kernel::g_object_pool.GetFast(archive_handle); + Archive* archive = Kernel::g_object_pool.Get(archive_handle); + if (archive == nullptr) { + return InvalidHandle(ErrorModule::FS); + } file->path = path; file->backend = archive->backend->OpenFile(path, mode); - if (!file->backend) - return 0; + if (!file->backend) { + return ResultCode(ErrorDescription::NotFound, ErrorModule::FS, + ErrorSummary::NotFound, ErrorLevel::Permanent); + } - return handle; + return MakeResult(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 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_handle); + Archive* archive = Kernel::g_object_pool.Get(archive_handle); + if (archive == nullptr) { + return InvalidHandle(ErrorModule::FS); + } directory->path = path; directory->backend = archive->backend->OpenDirectory(path); - return handle; + return MakeResult(handle); } /// Initialize archives diff --git a/src/core/hle/kernel/archive.h b/src/core/hle/kernel/archive.h index 95b3c6656..6fc4f0f25 100644 --- a/src/core/hle/kernel/archive.h +++ b/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 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 - * @return Handle to newly created Archive object + * @param name Name of Archive */ -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 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 OpenDirectoryFromArchive(Handle archive_handle, const FileSys::Path& path); /// Initialize archives void ArchiveInit(); diff --git a/src/core/hle/kernel/event.cpp b/src/core/hle/kernel/event.cpp index e0117c0bc..8a2925a3c 100644 --- a/src/core/hle/kernel/event.cpp +++ b/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 { - *wait = locked; + ResultVal WaitSynchronization() override { + 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(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) { - Event* evt = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!"); +ResultCode SetPermanentLock(Handle handle, const bool permanent_locked) { + Event* evt = g_object_pool.Get(handle); + 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) { - Event* evt = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!"); +ResultCode SetEventLocked(const Handle handle, const bool locked) { + Event* evt = g_object_pool.Get(handle); + 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) { - Event* evt = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!"); +ResultCode SignalEvent(const Handle handle) { + Event* evt = g_object_pool.Get(handle); + 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) { - Event* evt = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (evt != nullptr), "called, but event is nullptr!"); +ResultCode ClearEvent(Handle handle) { + Event* evt = g_object_pool.Get(handle); + if (evt == nullptr) return InvalidHandle(ErrorModule::Kernel); if (!evt->permanent_locked) { evt->locked = true; } - return 0; + return RESULT_SUCCESS; } /** diff --git a/src/core/hle/kernel/event.h b/src/core/hle/kernel/event.h index 6add72897..6c17ed232 100644 --- a/src/core/hle/kernel/event.h +++ b/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 diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index e0c94f186..8d3937ce8 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -7,6 +7,7 @@ #include #include #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 - * @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 + * Synchronize kernel object. + * @return True if the current thread should wait as a result of the sync */ - virtual Result SyncRequest(bool* wait) { + virtual ResultVal SyncRequest() { ERROR_LOG(KERNEL, "(UNIMPLEMENTED)"); - return -1; + return UnimplementedFunction(ErrorModule::Kernel); } /** - * Wait for kernel object to synchronize - * @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 + * Wait for kernel object to synchronize. + * @return True if the current thread should wait as a result of the wait */ - virtual Result WaitSynchronization(bool* wait) = 0; + virtual ResultVal WaitSynchronization() = 0; }; class ObjectPool : NonCopyable { @@ -80,38 +79,29 @@ public: static Object* CreateByIDType(int type); template - u32 Destroy(Handle handle) { - u32 error; - if (Get(handle, error)) { + void Destroy(Handle handle) { + if (Get(handle)) { occupied[handle - HANDLE_OFFSET] = false; delete pool[handle - HANDLE_OFFSET]; } - return error; } bool IsValid(Handle handle); template - 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 && (u32)handle != 0x80020001) { + if (handle != 0) { WARN_LOG(KERNEL, "Kernel: Bad object handle %i (%08x)", handle, handle); } - outError = 0;//T::GetMissingErrorCode(); - return 0; + return nullptr; } else { - // Previously we had a dynamic_cast here, but since RTTI was disabled traditionally, - // it just acted as a static case and everything worked. This means that we will never - // see the Wrong type object error below, but we'll just have to live with that danger. - T* t = static_cast(pool[handle - HANDLE_OFFSET]); - if (t == 0 || t->GetHandleType() != T::GetStaticHandleType()) { + Object* t = pool[handle - HANDLE_OFFSET]; + if (t->GetHandleType() != T::GetStaticHandleType()) { WARN_LOG(KERNEL, "Kernel: Wrong object type for %i (%08x)", handle, handle); - outError = 0;//T::GetMissingErrorCode(); - return 0; + return nullptr; } - outError = 0;//SCE_KERNEL_ERROR_OK; - return t; + return static_cast(t); } } diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 31129fd86..e4ff1ef40 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/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 SyncRequest() override { // TODO(bunnei): ImplementMe locked = true; - return 0; + return MakeResult(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 WaitSynchronization() override { // TODO(bunnei): ImplementMe - *wait = locked; - + bool wait = locked; if (locked) { Kernel::WaitCurrentThread(WAITTYPE_MUTEX, GetHandle()); } - return 0; + return MakeResult(wait); } }; @@ -119,15 +118,17 @@ bool ReleaseMutex(Mutex* mutex) { * Releases a mutex * @param handle Handle to mutex to release */ -Result ReleaseMutex(Handle handle) { - Mutex* mutex = Kernel::g_object_pool.GetFast(handle); - - _assert_msg_(KERNEL, (mutex != nullptr), "ReleaseMutex tried to release a nullptr mutex!"); +ResultCode ReleaseMutex(Handle handle) { + Mutex* mutex = Kernel::g_object_pool.Get(handle); + if (mutex == nullptr) return InvalidHandle(ErrorModule::Kernel); 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; } /** diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h index 313ba6fee..233d8c420 100644 --- a/src/core/hle/kernel/mutex.h +++ b/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 diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index 7ef3e54cc..b91fc98da 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/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 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(handle); - _assert_msg_(KERNEL, (shared_memory != nullptr), "handle 0x%08X is not valid!", handle); + SharedMemory* shared_memory = Kernel::g_object_pool.Get(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) { - SharedMemory* shared_memory = Kernel::g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (shared_memory != nullptr), "handle 0x%08X is not valid!", handle); +ResultVal GetSharedMemoryPointer(Handle handle, u32 offset) { + SharedMemory* shared_memory = Kernel::g_object_pool.Get(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(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 diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index 0aec03538..6ed427088 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/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 GetSharedMemoryPointer(Handle handle, u32 offset); } // namespace diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index cc70cbca7..b01779f2e 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/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 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(true); + } else { + return MakeResult(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) { - Thread* thread = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); +inline bool VerifyWait(Handle handle, WaitType type, Handle wait_handle) { + Thread* thread = g_object_pool.Get(handle); + _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) { - Thread* thread = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); +ResultCode StopThread(Handle handle, const char* reason) { + Thread* thread = g_object_pool.Get(handle); + 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(handle); + Thread* thread = g_object_pool.Get(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(next); + return Kernel::g_object_pool.Get(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(handle, error); + Thread* thread = Kernel::g_object_pool.Get(handle); 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) { - Thread* thread = g_object_pool.GetFast(handle); - _assert_msg_(KERNEL, (thread != nullptr), "called, but thread is nullptr!"); - return thread->current_priority; +ResultVal GetThreadPriority(const Handle handle) { + Thread* thread = g_object_pool.Get(handle); + if (thread == nullptr) return InvalidHandle(ErrorModule::Kernel); + + return MakeResult(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(handle); + thread = g_object_pool.Get(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 diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 2a43797ee..ce63a70d3 100644 --- a/src/core/hle/kernel/thread.h +++ b/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 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(); diff --git a/src/core/hle/result.h b/src/core/hle/result.h new file mode 100644 index 000000000..15c4a2677 --- /dev/null +++ b/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 +#include +#include +#include + +#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 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(42); + * } + * } + * \endcode + * + * \code + * ResultVal 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 +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 + static ResultVal WithCode(ResultCode success_code, Args&&... args) { + ResultVal result; + result.emplace(success_code, std::forward(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 + void emplace(ResultCode success_code, Args&&... args) { + assert(success_code.IsSuccess()); + if (!empty()) { + GetPointer()->~T(); + } + new (&storage) T(std::forward(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 + T ValueOr(U&& value) const { + return !empty() ? *GetPointer() : std::move(value); + } + +private: + typedef typename std::aligned_storage::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(static_cast(&storage)); +#endif + } + + const T* GetPointer() const { + assert(!empty()); + return static_cast(static_cast(&storage)); + } + + T* GetPointer() { + assert(!empty()); + return static_cast(static_cast(&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 +ResultVal MakeResult(Args&&... args) { + return ResultVal::WithCode(RESULT_SUCCESS, std::forward(args)...); +} diff --git a/src/core/hle/service/fs_user.cpp b/src/core/hle/service/fs_user.cpp index 2aec1a52a..435be5b5d 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 = 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 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 = 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 = 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 = 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 66daded94..de1bd3f61 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 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 ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, offset); + return reinterpret_cast(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 ptr = Kernel::GetSharedMemoryPointer(g_shared_memory, sizeof(InterruptRelayQueue) * thread_id); + return reinterpret_cast(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 5f6bf1eff..d29de1a52 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(Kernel::GetSharedMemoryPointer(shared_mem, 0)); + return reinterpret_cast(Kernel::GetSharedMemoryPointer(shared_mem, 0).ValueOr(nullptr)); } /** diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index abc8d5edb..fed2268a0 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(handle); + return Kernel::g_object_pool.Get(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 55aa84e83..136984b93 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 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(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(false); } itr->second.func(this); - return 0; // TODO: Implement return from actual function + return MakeResult(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 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 df38bd93c..0e7fa9e3b 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[] = { diff --git a/src/core/hle/svc.cpp b/src/core/hle/svc.cpp index 3a06d6765..87d768856 100644 --- a/src/core/hle/svc.cpp +++ b/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(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; - Result res = object->SyncRequest(&wait); - if (wait) { + ResultVal wait = object->SyncRequest(); + if (wait.Succeeded() && *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(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!"); - - Result res = object->WaitSynchronization(&wait); + ResultVal wait = object->WaitSynchronization(); // 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(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 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(type), address, - value); + return Kernel::ArbitrateAddress(arbiter, static_cast(type), + 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); - return 0; + ResultVal priority_result = Kernel::GetThreadPriority(handle); + 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!"); - Kernel::ReleaseMutex(handle); - return 0; + ResultCode res = Kernel::ReleaseMutex(handle); + return res.raw; } /// 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 From 22c86824a4e4fe8953d7104f2fbdf853d3d30c60 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 31 Oct 2014 21:19:20 -0200 Subject: [PATCH 3/4] Remove duplicated docs/update them for changed parameters. --- src/core/hle/kernel/address_arbiter.cpp | 5 ---- src/core/hle/kernel/archive.cpp | 31 ------------------------- src/core/hle/kernel/event.cpp | 5 ---- src/core/hle/kernel/event.h | 4 ---- src/core/hle/kernel/mutex.cpp | 10 -------- src/core/hle/kernel/mutex.h | 1 - src/core/hle/kernel/shared_memory.cpp | 16 ------------- src/core/hle/kernel/shared_memory.h | 1 - src/core/hle/kernel/thread.cpp | 5 ---- src/core/hle/service/service.h | 10 -------- 10 files changed, 88 deletions(-) diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp index 1e697fac1..db571b895 100644 --- a/src/core/hle/kernel/address_arbiter.cpp +++ b/src/core/hle/kernel/address_arbiter.cpp @@ -25,11 +25,6 @@ public: std::string name; ///< Name of address arbiter object (optional) - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "(UNIMPLEMENTED)"); diff --git a/src/core/hle/kernel/archive.cpp b/src/core/hle/kernel/archive.cpp index e11dddc84..e273444c9 100644 --- a/src/core/hle/kernel/archive.cpp +++ b/src/core/hle/kernel/archive.cpp @@ -52,11 +52,6 @@ public: std::string name; ///< Name of archive (optional) FileSys::Archive* backend; ///< Archive backend interface - /** - * Synchronize kernel object - * @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 - */ ResultVal SyncRequest() override { u32* cmd_buff = Service::GetCommandBuffer(); FileCommand cmd = static_cast(cmd_buff[0]); @@ -114,11 +109,6 @@ public: return MakeResult(false); } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "(UNIMPLEMENTED)"); @@ -137,11 +127,6 @@ public: FileSys::Path path; ///< Path of the file std::unique_ptr backend; ///< File backend interface - /** - * Synchronize kernel object - * @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 - */ ResultVal SyncRequest() override { u32* cmd_buff = Service::GetCommandBuffer(); FileCommand cmd = static_cast(cmd_buff[0]); @@ -208,11 +193,6 @@ public: return MakeResult(false); } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "(UNIMPLEMENTED)"); @@ -231,11 +211,6 @@ public: FileSys::Path path; ///< Path of the directory std::unique_ptr backend; ///< File backend interface - /** - * Synchronize kernel object - * @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 - */ ResultVal SyncRequest() override { u32* cmd_buff = Service::GetCommandBuffer(); DirectoryCommand cmd = static_cast(cmd_buff[0]); @@ -273,11 +248,6 @@ public: return MakeResult(false); } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "(UNIMPLEMENTED)"); @@ -313,7 +283,6 @@ ResultCode CloseArchive(FileSys::Archive::IdCode id_code) { /** * Mounts an archive * @param archive Pointer to the archive to mount - * @return Result of operation */ ResultCode MountArchive(Archive* archive) { FileSys::Archive::IdCode id_code = archive->backend->GetIdCode(); diff --git a/src/core/hle/kernel/event.cpp b/src/core/hle/kernel/event.cpp index 8a2925a3c..288080209 100644 --- a/src/core/hle/kernel/event.cpp +++ b/src/core/hle/kernel/event.cpp @@ -30,11 +30,6 @@ public: std::vector waiting_threads; ///< Threads that are waiting for the event std::string name; ///< Name of event (optional) - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { bool wait = locked; if (locked) { diff --git a/src/core/hle/kernel/event.h b/src/core/hle/kernel/event.h index 6c17ed232..73aec4e79 100644 --- a/src/core/hle/kernel/event.h +++ b/src/core/hle/kernel/event.h @@ -15,7 +15,6 @@ namespace Kernel { * Changes whether an event is locked or not * @param handle Handle to event to change * @param locked Boolean locked value to set event - * @return Result of operation, 0 on success, otherwise error code */ ResultCode SetEventLocked(const Handle handle, const bool locked); @@ -23,21 +22,18 @@ ResultCode SetEventLocked(const Handle handle, const bool locked); * Hackish function to set an events permanent lock state, used to pass through synch blocks * @param handle Handle to event to change * @param permanent_locked Boolean permanent locked value to set event - * @return Result of operation, 0 on success, otherwise error code */ 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 */ 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 */ ResultCode ClearEvent(Handle handle); diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index e4ff1ef40..b303ba128 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -27,22 +27,12 @@ public: std::vector waiting_threads; ///< Threads that are waiting for the mutex std::string name; ///< Name of mutex (optional) - /** - * Synchronize kernel object - * @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 - */ ResultVal SyncRequest() override { // TODO(bunnei): ImplementMe locked = true; return MakeResult(false); } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe bool wait = locked; diff --git a/src/core/hle/kernel/mutex.h b/src/core/hle/kernel/mutex.h index 233d8c420..155449f95 100644 --- a/src/core/hle/kernel/mutex.h +++ b/src/core/hle/kernel/mutex.h @@ -13,7 +13,6 @@ namespace Kernel { /** * Releases a mutex * @param handle Handle to mutex to release - * @return Result of operation, 0 on success, otherwise error code */ ResultCode ReleaseMutex(Handle handle); diff --git a/src/core/hle/kernel/shared_memory.cpp b/src/core/hle/kernel/shared_memory.cpp index b91fc98da..cfcc0e0b7 100644 --- a/src/core/hle/kernel/shared_memory.cpp +++ b/src/core/hle/kernel/shared_memory.cpp @@ -16,11 +16,6 @@ public: static Kernel::HandleType GetStaticHandleType() { return Kernel::HandleType::SharedMemory; } Kernel::HandleType GetHandleType() const override { return Kernel::HandleType::SharedMemory; } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "(UNIMPLEMENTED)"); @@ -48,11 +43,6 @@ SharedMemory* CreateSharedMemory(Handle& handle, const std::string& name) { return shared_memory; } -/** - * Creates a shared memory object - * @param name Optional name of shared memory object - * @return Handle of newly created shared memory object - */ Handle CreateSharedMemory(const std::string& name) { Handle handle; CreateSharedMemory(handle, name); @@ -86,12 +76,6 @@ ResultCode MapSharedMemory(u32 handle, u32 address, MemoryPermission permissions return RESULT_SUCCESS; } -/** - * Gets a pointer to the shared memory block - * @param handle Shared memory block handle - * @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 - */ ResultVal GetSharedMemoryPointer(Handle handle, u32 offset) { SharedMemory* shared_memory = Kernel::g_object_pool.Get(handle); if (shared_memory == nullptr) return InvalidHandle(ErrorModule::Kernel); diff --git a/src/core/hle/kernel/shared_memory.h b/src/core/hle/kernel/shared_memory.h index 6ed427088..304cf5b67 100644 --- a/src/core/hle/kernel/shared_memory.h +++ b/src/core/hle/kernel/shared_memory.h @@ -32,7 +32,6 @@ Handle CreateSharedMemory(const std::string& name="Unknown"); * @param address Address in system memory to map shared memory block to * @param permissions Memory block map permissions (specified by SVC field) * @param other_permissions Memory block map other permissions (specified by SVC field) - * @return Result of operation, 0 on success, otherwise error code */ ResultCode MapSharedMemory(Handle handle, u32 address, MemoryPermission permissions, MemoryPermission other_permissions); diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index b01779f2e..2521f883d 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -34,11 +34,6 @@ public: inline bool IsWaiting() const { return (status & THREADSTATUS_WAIT) != 0; } inline bool IsSuspended() const { return (status & THREADSTATUS_SUSPEND) != 0; } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { if (status != THREADSTATUS_DORMANT) { Handle thread = GetCurrentThreadHandle(); diff --git a/src/core/hle/service/service.h b/src/core/hle/service/service.h index 136984b93..20e7fb4d3 100644 --- a/src/core/hle/service/service.h +++ b/src/core/hle/service/service.h @@ -75,11 +75,6 @@ public: m_handles.erase(std::remove(m_handles.begin(), m_handles.end(), handle), m_handles.end()); } - /** - * Synchronize kernel object - * @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 - */ ResultVal SyncRequest() override { u32* cmd_buff = GetCommandBuffer(); auto itr = m_functions.find(cmd_buff[0]); @@ -108,11 +103,6 @@ public: return MakeResult(false); // TODO: Implement return from actual function } - /** - * Wait for kernel object to synchronize - * @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 - */ ResultVal WaitSynchronization() override { // TODO(bunnei): ImplementMe ERROR_LOG(OSHLE, "unimplemented function"); From 8189593255df8ab4abb699082f2c48baa3b0656b Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Mon, 24 Nov 2014 15:51:50 -0200 Subject: [PATCH 4/4] Use pointers instead of passing handles around in some functions. --- src/core/hle/kernel/thread.cpp | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 2521f883d..f3f54a4e9 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -35,16 +35,16 @@ public: inline bool IsSuspended() const { return (status & THREADSTATUS_SUSPEND) != 0; } ResultVal WaitSynchronization() override { - if (status != THREADSTATUS_DORMANT) { + const bool wait = status != THREADSTATUS_DORMANT; + if (wait) { 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()); - return MakeResult(true); - } else { - return MakeResult(false); } + + return MakeResult(wait); } ThreadContext context; @@ -141,14 +141,9 @@ void ChangeReadyState(Thread* t, bool ready) { } /// Verify that a thread has not been released from waiting -inline bool VerifyWait(Handle handle, WaitType type, Handle wait_handle) { - Thread* thread = g_object_pool.Get(handle); +inline bool VerifyWait(const Thread* thread, WaitType type, Handle wait_handle) { _dbg_assert_(KERNEL, thread != nullptr); - - if (type != thread->wait_type || wait_handle != thread->wait_handle) - return false; - - return true; + return type == thread->wait_type && wait_handle == thread->wait_handle; } /// Stops the current thread @@ -158,10 +153,10 @@ ResultCode StopThread(Handle handle, const char* reason) { ChangeReadyState(thread, false); thread->status = THREADSTATUS_DORMANT; - for (size_t i = 0; i < thread->waiting_threads.size(); ++i) { - const Handle waiting_thread = thread->waiting_threads[i]; + for (Handle waiting_handle : thread->waiting_threads) { + Thread* waiting_thread = g_object_pool.Get(waiting_handle); if (VerifyWait(waiting_thread, WAITTYPE_THREADEND, handle)) { - ResumeThreadFromWait(waiting_thread); + ResumeThreadFromWait(waiting_handle); } } thread->waiting_threads.clear(); @@ -194,13 +189,13 @@ Handle ArbitrateHighestPriorityThread(u32 arbiter, u32 address) { s32 priority = THREADPRIO_LOWEST; // Iterate through threads, find highest priority thread that is waiting to be arbitrated... - for (const auto& handle : thread_queue) { + for (Handle handle : thread_queue) { + Thread* thread = g_object_pool.Get(handle); // TODO(bunnei): Verify arbiter address... - if (!VerifyWait(handle, WAITTYPE_ARB, arbiter)) + if (!VerifyWait(thread, WAITTYPE_ARB, arbiter)) continue; - Thread* thread = g_object_pool.Get(handle); if (thread == nullptr) continue; // TODO(yuriks): Thread handle will hang around forever. Should clean up. if(thread->current_priority <= priority) { @@ -219,10 +214,11 @@ Handle ArbitrateHighestPriorityThread(u32 arbiter, u32 address) { void ArbitrateAllThreads(u32 arbiter, u32 address) { // Iterate through threads, find highest priority thread that is waiting to be arbitrated... - for (const auto& handle : thread_queue) { + for (Handle handle : thread_queue) { + Thread* thread = g_object_pool.Get(handle); // TODO(bunnei): Verify arbiter address... - if (VerifyWait(handle, WAITTYPE_ARB, arbiter)) + if (VerifyWait(thread, WAITTYPE_ARB, arbiter)) ResumeThreadFromWait(handle); } }