From c31f19b6d1468e4a8a4d3a337575599e1d235b5d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 23 Nov 2022 13:08:49 -0500 Subject: [PATCH 1/3] hle_ipc: Mark relevant member functions as [[nodiscard]] Will allow the compiler to complain about cases where ignoring the return value would be a bug. --- src/core/hle/kernel/hle_ipc.h | 50 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index d87be72d6..d68ea5b1e 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -199,7 +199,7 @@ public: ~HLERequestContext(); /// Returns a pointer to the IPC command buffer for this request. - u32* CommandBuffer() { + [[nodiscard]] u32* CommandBuffer() { return cmd_buf.data(); } @@ -207,7 +207,7 @@ public: * Returns the session through which this request was made. This can be used as a map key to * access per-client data on services. */ - Kernel::KServerSession* Session() { + [[nodiscard]] Kernel::KServerSession* Session() { return server_session; } @@ -217,61 +217,61 @@ public: /// Writes data from this context back to the requesting process/thread. Result WriteToOutgoingCommandBuffer(KThread& requesting_thread); - u32_le GetHipcCommand() const { + [[nodiscard]] u32_le GetHipcCommand() const { return command; } - u32_le GetTipcCommand() const { + [[nodiscard]] u32_le GetTipcCommand() const { return static_cast(command_header->type.Value()) - static_cast(IPC::CommandType::TIPC_CommandRegion); } - u32_le GetCommand() const { + [[nodiscard]] u32_le GetCommand() const { return command_header->IsTipc() ? GetTipcCommand() : GetHipcCommand(); } - bool IsTipc() const { + [[nodiscard]] bool IsTipc() const { return command_header->IsTipc(); } - IPC::CommandType GetCommandType() const { + [[nodiscard]] IPC::CommandType GetCommandType() const { return command_header->type; } - u64 GetPID() const { + [[nodiscard]] u64 GetPID() const { return pid; } - u32 GetDataPayloadOffset() const { + [[nodiscard]] u32 GetDataPayloadOffset() const { return data_payload_offset; } - const std::vector& BufferDescriptorX() const { + [[nodiscard]] const std::vector& BufferDescriptorX() const { return buffer_x_desciptors; } - const std::vector& BufferDescriptorA() const { + [[nodiscard]] const std::vector& BufferDescriptorA() const { return buffer_a_desciptors; } - const std::vector& BufferDescriptorB() const { + [[nodiscard]] const std::vector& BufferDescriptorB() const { return buffer_b_desciptors; } - const std::vector& BufferDescriptorC() const { + [[nodiscard]] const std::vector& BufferDescriptorC() const { return buffer_c_desciptors; } - const IPC::DomainMessageHeader& GetDomainMessageHeader() const { + [[nodiscard]] const IPC::DomainMessageHeader& GetDomainMessageHeader() const { return domain_message_header.value(); } - bool HasDomainMessageHeader() const { + [[nodiscard]] bool HasDomainMessageHeader() const { return domain_message_header.has_value(); } /// Helper function to read a buffer using the appropriate buffer descriptor - std::vector ReadBuffer(std::size_t buffer_index = 0) const; + [[nodiscard]] std::vector ReadBuffer(std::size_t buffer_index = 0) const; /// Helper function to write a buffer using the appropriate buffer descriptor std::size_t WriteBuffer(const void* buffer, std::size_t size, @@ -308,22 +308,22 @@ public: } /// Helper function to get the size of the input buffer - std::size_t GetReadBufferSize(std::size_t buffer_index = 0) const; + [[nodiscard]] std::size_t GetReadBufferSize(std::size_t buffer_index = 0) const; /// Helper function to get the size of the output buffer - std::size_t GetWriteBufferSize(std::size_t buffer_index = 0) const; + [[nodiscard]] std::size_t GetWriteBufferSize(std::size_t buffer_index = 0) const; /// Helper function to test whether the input buffer at buffer_index can be read - bool CanReadBuffer(std::size_t buffer_index = 0) const; + [[nodiscard]] bool CanReadBuffer(std::size_t buffer_index = 0) const; /// Helper function to test whether the output buffer at buffer_index can be written - bool CanWriteBuffer(std::size_t buffer_index = 0) const; + [[nodiscard]] bool CanWriteBuffer(std::size_t buffer_index = 0) const; - Handle GetCopyHandle(std::size_t index) const { + [[nodiscard]] Handle GetCopyHandle(std::size_t index) const { return incoming_copy_handles.at(index); } - Handle GetMoveHandle(std::size_t index) const { + [[nodiscard]] Handle GetMoveHandle(std::size_t index) const { return incoming_move_handles.at(index); } @@ -348,13 +348,13 @@ public: manager = manager_; } - std::string Description() const; + [[nodiscard]] std::string Description() const; - KThread& GetThread() { + [[nodiscard]] KThread& GetThread() { return *thread; } - std::shared_ptr GetManager() const { + [[nodiscard]] std::shared_ptr GetManager() const { return manager.lock(); } From 59335f6796480f6b98f981cbb672cdb112a8a3f8 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 23 Nov 2022 13:14:04 -0500 Subject: [PATCH 2/3] hle_ipc: Add helper functions for getting number of buffer elements --- src/core/hle/kernel/hle_ipc.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index d68ea5b1e..e252b5f4b 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -313,6 +313,18 @@ public: /// Helper function to get the size of the output buffer [[nodiscard]] std::size_t GetWriteBufferSize(std::size_t buffer_index = 0) const; + /// Helper function to derive the number of elements able to be contained in the read buffer + template + [[nodiscard]] std::size_t GetReadBufferNumElements(std::size_t buffer_index = 0) const { + return GetReadBufferSize(buffer_index) / sizeof(T); + } + + /// Helper function to derive the number of elements able to be contained in the write buffer + template + [[nodiscard]] std::size_t GetWriteBufferNumElements(std::size_t buffer_index = 0) const { + return GetWriteBufferSize(buffer_index) / sizeof(T); + } + /// Helper function to test whether the input buffer at buffer_index can be read [[nodiscard]] bool CanReadBuffer(std::size_t buffer_index = 0) const; From 97f273e94e83a679f42faa9c81916a1c058112e1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 23 Nov 2022 13:25:14 -0500 Subject: [PATCH 3/3] service: Make use of buffer element count helpers --- src/core/hle/service/audio/audin_u.cpp | 10 +++---- src/core/hle/service/audio/audout_u.cpp | 10 +++---- src/core/hle/service/audio/audren_u.cpp | 6 ++--- src/core/hle/service/audio/hwopus.cpp | 2 +- src/core/hle/service/bcat/bcat_module.cpp | 4 +-- src/core/hle/service/es/es.cpp | 27 +++++++++---------- src/core/hle/service/filesystem/fsp_srv.cpp | 4 +-- src/core/hle/service/ldn/ldn.cpp | 4 +-- src/core/hle/service/nfc/nfc_user.cpp | 2 +- src/core/hle/service/nfp/nfp_user.cpp | 6 ++--- .../service/ns/iplatform_service_manager.cpp | 11 +++----- src/core/hle/service/set/set.cpp | 2 +- 12 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/core/hle/service/audio/audin_u.cpp b/src/core/hle/service/audio/audin_u.cpp index 608925dfc..053e8f9dd 100644 --- a/src/core/hle/service/audio/audin_u.cpp +++ b/src/core/hle/service/audio/audin_u.cpp @@ -122,10 +122,10 @@ private: } void GetReleasedAudioInBuffer(Kernel::HLERequestContext& ctx) { - auto write_buffer_size = ctx.GetWriteBufferSize() / sizeof(u64); - std::vector released_buffers(write_buffer_size, 0); + const auto write_buffer_size = ctx.GetWriteBufferNumElements(); + std::vector released_buffers(write_buffer_size); - auto count = impl->GetReleasedBuffers(released_buffers); + const auto count = impl->GetReleasedBuffers(released_buffers); [[maybe_unused]] std::string tags{}; for (u32 i = 0; i < count; i++) { @@ -228,7 +228,7 @@ void AudInU::ListAudioIns(Kernel::HLERequestContext& ctx) { LOG_DEBUG(Service_Audio, "called"); const auto write_count = - static_cast(ctx.GetWriteBufferSize() / sizeof(AudioDevice::AudioDeviceName)); + static_cast(ctx.GetWriteBufferNumElements()); std::vector device_names{}; u32 out_count{0}; @@ -248,7 +248,7 @@ void AudInU::ListAudioInsAutoFiltered(Kernel::HLERequestContext& ctx) { LOG_DEBUG(Service_Audio, "called"); const auto write_count = - static_cast(ctx.GetWriteBufferSize() / sizeof(AudioDevice::AudioDeviceName)); + static_cast(ctx.GetWriteBufferNumElements()); std::vector device_names{}; u32 out_count{0}; diff --git a/src/core/hle/service/audio/audout_u.cpp b/src/core/hle/service/audio/audout_u.cpp index 122290c6a..29751f075 100644 --- a/src/core/hle/service/audio/audout_u.cpp +++ b/src/core/hle/service/audio/audout_u.cpp @@ -129,16 +129,16 @@ private: } void GetReleasedAudioOutBuffers(Kernel::HLERequestContext& ctx) { - auto write_buffer_size = ctx.GetWriteBufferSize() / sizeof(u64); - std::vector released_buffers(write_buffer_size, 0); + const auto write_buffer_size = ctx.GetWriteBufferNumElements(); + std::vector released_buffers(write_buffer_size); - auto count = impl->GetReleasedBuffers(released_buffers); + const auto count = impl->GetReleasedBuffers(released_buffers); [[maybe_unused]] std::string tags{}; for (u32 i = 0; i < count; i++) { tags += fmt::format("{:08X}, ", released_buffers[i]); } - [[maybe_unused]] auto sessionid{impl->GetSystem().GetSessionId()}; + [[maybe_unused]] const auto sessionid{impl->GetSystem().GetSessionId()}; LOG_TRACE(Service_Audio, "called. Session {} released {} buffers: {}", sessionid, count, tags); @@ -244,7 +244,7 @@ void AudOutU::ListAudioOuts(Kernel::HLERequestContext& ctx) { std::scoped_lock l{impl->mutex}; const auto write_count = - static_cast(ctx.GetWriteBufferSize() / sizeof(AudioDevice::AudioDeviceName)); + static_cast(ctx.GetWriteBufferNumElements()); std::vector device_names{}; if (write_count > 0) { device_names.emplace_back("DeviceOut"); diff --git a/src/core/hle/service/audio/audren_u.cpp b/src/core/hle/service/audio/audren_u.cpp index 13423dca6..034ee273f 100644 --- a/src/core/hle/service/audio/audren_u.cpp +++ b/src/core/hle/service/audio/audren_u.cpp @@ -274,7 +274,7 @@ public: private: void ListAudioDeviceName(Kernel::HLERequestContext& ctx) { - const size_t in_count = ctx.GetWriteBufferSize() / sizeof(AudioDevice::AudioDeviceName); + const size_t in_count = ctx.GetWriteBufferNumElements(); std::vector out_names{}; @@ -335,7 +335,7 @@ private: } void GetActiveAudioDeviceName(Kernel::HLERequestContext& ctx) { - const auto write_size = ctx.GetWriteBufferSize() / sizeof(char); + const auto write_size = ctx.GetWriteBufferSize(); std::string out_name{"AudioTvOutput"}; LOG_DEBUG(Service_Audio, "(STUBBED) called. Name={}", out_name); @@ -387,7 +387,7 @@ private: } void ListAudioOutputDeviceName(Kernel::HLERequestContext& ctx) { - const size_t in_count = ctx.GetWriteBufferSize() / sizeof(AudioDevice::AudioDeviceName); + const size_t in_count = ctx.GetWriteBufferNumElements(); std::vector out_names{}; diff --git a/src/core/hle/service/audio/hwopus.cpp b/src/core/hle/service/audio/hwopus.cpp index 8bafc3a98..825fb8bcc 100644 --- a/src/core/hle/service/audio/hwopus.cpp +++ b/src/core/hle/service/audio/hwopus.cpp @@ -68,7 +68,7 @@ private: ExtraBehavior extra_behavior) { u32 consumed = 0; u32 sample_count = 0; - std::vector samples(ctx.GetWriteBufferSize() / sizeof(opus_int16)); + std::vector samples(ctx.GetWriteBufferNumElements()); if (extra_behavior == ExtraBehavior::ResetContext) { ResetDecoderContext(); diff --git a/src/core/hle/service/bcat/bcat_module.cpp b/src/core/hle/service/bcat/bcat_module.cpp index bc08ac487..cbe690a5d 100644 --- a/src/core/hle/service/bcat/bcat_module.cpp +++ b/src/core/hle/service/bcat/bcat_module.cpp @@ -443,7 +443,7 @@ private: } void Read(Kernel::HLERequestContext& ctx) { - auto write_size = ctx.GetWriteBufferSize() / sizeof(DeliveryCacheDirectoryEntry); + auto write_size = ctx.GetWriteBufferNumElements(); LOG_DEBUG(Service_BCAT, "called, write_size={:016X}", write_size); @@ -533,7 +533,7 @@ private: } void EnumerateDeliveryCacheDirectory(Kernel::HLERequestContext& ctx) { - auto size = ctx.GetWriteBufferSize() / sizeof(DirectoryName); + auto size = ctx.GetWriteBufferNumElements(); LOG_DEBUG(Service_BCAT, "called, size={:016X}", size); diff --git a/src/core/hle/service/es/es.cpp b/src/core/hle/service/es/es.cpp index ff9b0427c..d183e5829 100644 --- a/src/core/hle/service/es/es.cpp +++ b/src/core/hle/service/es/es.cpp @@ -192,12 +192,10 @@ private: } void ListCommonTicketRightsIds(Kernel::HLERequestContext& ctx) { - u32 out_entries; - if (keys.GetCommonTickets().empty()) - out_entries = 0; - else - out_entries = static_cast(ctx.GetWriteBufferSize() / sizeof(u128)); - + size_t out_entries = 0; + if (!keys.GetCommonTickets().empty()) { + out_entries = ctx.GetWriteBufferNumElements(); + } LOG_DEBUG(Service_ETicket, "called, entries={:016X}", out_entries); keys.PopulateTickets(); @@ -206,20 +204,19 @@ private: std::transform(tickets.begin(), tickets.end(), std::back_inserter(ids), [](const auto& pair) { return pair.first; }); - out_entries = static_cast(std::min(ids.size(), out_entries)); + out_entries = std::min(ids.size(), out_entries); ctx.WriteBuffer(ids.data(), out_entries * sizeof(u128)); IPC::ResponseBuilder rb{ctx, 3}; rb.Push(ResultSuccess); - rb.Push(out_entries); + rb.Push(static_cast(out_entries)); } void ListPersonalizedTicketRightsIds(Kernel::HLERequestContext& ctx) { - u32 out_entries; - if (keys.GetPersonalizedTickets().empty()) - out_entries = 0; - else - out_entries = static_cast(ctx.GetWriteBufferSize() / sizeof(u128)); + size_t out_entries = 0; + if (!keys.GetPersonalizedTickets().empty()) { + out_entries = ctx.GetWriteBufferNumElements(); + } LOG_DEBUG(Service_ETicket, "called, entries={:016X}", out_entries); @@ -229,12 +226,12 @@ private: std::transform(tickets.begin(), tickets.end(), std::back_inserter(ids), [](const auto& pair) { return pair.first; }); - out_entries = static_cast(std::min(ids.size(), out_entries)); + out_entries = std::min(ids.size(), out_entries); ctx.WriteBuffer(ids.data(), out_entries * sizeof(u128)); IPC::ResponseBuilder rb{ctx, 3}; rb.Push(ResultSuccess); - rb.Push(out_entries); + rb.Push(static_cast(out_entries)); } void GetCommonTicketSize(Kernel::HLERequestContext& ctx) { diff --git a/src/core/hle/service/filesystem/fsp_srv.cpp b/src/core/hle/service/filesystem/fsp_srv.cpp index c08274ef9..fbb16a7da 100644 --- a/src/core/hle/service/filesystem/fsp_srv.cpp +++ b/src/core/hle/service/filesystem/fsp_srv.cpp @@ -277,7 +277,7 @@ private: LOG_DEBUG(Service_FS, "called."); // Calculate how many entries we can fit in the output buffer - const u64 count_entries = ctx.GetWriteBufferSize() / sizeof(FileSys::Entry); + const u64 count_entries = ctx.GetWriteBufferNumElements(); // Cap at total number of entries. const u64 actual_entries = std::min(count_entries, entries.size() - next_entry_index); @@ -543,7 +543,7 @@ public: LOG_DEBUG(Service_FS, "called"); // Calculate how many entries we can fit in the output buffer - const u64 count_entries = ctx.GetWriteBufferSize() / sizeof(SaveDataInfo); + const u64 count_entries = ctx.GetWriteBufferNumElements(); // Cap at total number of entries. const u64 actual_entries = std::min(count_entries, info.size() - next_entry_index); diff --git a/src/core/hle/service/ldn/ldn.cpp b/src/core/hle/service/ldn/ldn.cpp index 6df563136..c49c61cff 100644 --- a/src/core/hle/service/ldn/ldn.cpp +++ b/src/core/hle/service/ldn/ldn.cpp @@ -292,7 +292,7 @@ public: void GetNetworkInfoLatestUpdate(Kernel::HLERequestContext& ctx) { const std::size_t network_buffer_size = ctx.GetWriteBufferSize(0); - const std::size_t node_buffer_count = ctx.GetWriteBufferSize(1) / sizeof(NodeLatestUpdate); + const std::size_t node_buffer_count = ctx.GetWriteBufferNumElements(1); if (node_buffer_count == 0 || network_buffer_size != sizeof(NetworkInfo)) { LOG_ERROR(Service_LDN, "Invalid buffer, size = {}, count = {}", network_buffer_size, @@ -333,7 +333,7 @@ public: const auto channel{rp.PopEnum()}; const auto scan_filter{rp.PopRaw()}; - const std::size_t network_info_size = ctx.GetWriteBufferSize() / sizeof(NetworkInfo); + const std::size_t network_info_size = ctx.GetWriteBufferNumElements(); if (network_info_size == 0) { LOG_ERROR(Service_LDN, "Invalid buffer size {}", network_info_size); diff --git a/src/core/hle/service/nfc/nfc_user.cpp b/src/core/hle/service/nfc/nfc_user.cpp index 0753333bf..ced2d560b 100644 --- a/src/core/hle/service/nfc/nfc_user.cpp +++ b/src/core/hle/service/nfc/nfc_user.cpp @@ -118,7 +118,7 @@ void IUser::ListDevices(Kernel::HLERequestContext& ctx) { } std::vector nfp_devices; - const std::size_t max_allowed_devices = ctx.GetWriteBufferSize() / sizeof(u64); + const std::size_t max_allowed_devices = ctx.GetWriteBufferNumElements(); for (auto& device : devices) { if (nfp_devices.size() >= max_allowed_devices) { diff --git a/src/core/hle/service/nfp/nfp_user.cpp b/src/core/hle/service/nfp/nfp_user.cpp index 2fe3c0ea0..49816b4c7 100644 --- a/src/core/hle/service/nfp/nfp_user.cpp +++ b/src/core/hle/service/nfp/nfp_user.cpp @@ -104,9 +104,9 @@ void IUser::ListDevices(Kernel::HLERequestContext& ctx) { } std::vector nfp_devices; - const std::size_t max_allowed_devices = ctx.GetWriteBufferSize() / sizeof(u64); + const std::size_t max_allowed_devices = ctx.GetWriteBufferNumElements(); - for (auto& device : devices) { + for (const auto& device : devices) { if (nfp_devices.size() >= max_allowed_devices) { continue; } @@ -115,7 +115,7 @@ void IUser::ListDevices(Kernel::HLERequestContext& ctx) { } } - if (nfp_devices.size() == 0) { + if (nfp_devices.empty()) { IPC::ResponseBuilder rb{ctx, 2}; rb.Push(DeviceNotFound); return; diff --git a/src/core/hle/service/ns/iplatform_service_manager.cpp b/src/core/hle/service/ns/iplatform_service_manager.cpp index fd047ff26..1fab2f0dd 100644 --- a/src/core/hle/service/ns/iplatform_service_manager.cpp +++ b/src/core/hle/service/ns/iplatform_service_manager.cpp @@ -279,13 +279,10 @@ void IPlatformServiceManager::GetSharedFontInOrderOfPriority(Kernel::HLERequestC font_sizes.push_back(region.size); } - // Resize buffers if game requests smaller size output. - font_codes.resize( - std::min(font_codes.size(), ctx.GetWriteBufferSize(0) / sizeof(u32))); - font_offsets.resize( - std::min(font_offsets.size(), ctx.GetWriteBufferSize(1) / sizeof(u32))); - font_sizes.resize( - std::min(font_sizes.size(), ctx.GetWriteBufferSize(2) / sizeof(u32))); + // Resize buffers if game requests smaller size output + font_codes.resize(std::min(font_codes.size(), ctx.GetWriteBufferNumElements(0))); + font_offsets.resize(std::min(font_offsets.size(), ctx.GetWriteBufferNumElements(1))); + font_sizes.resize(std::min(font_sizes.size(), ctx.GetWriteBufferNumElements(2))); ctx.WriteBuffer(font_codes, 0); ctx.WriteBuffer(font_offsets, 1); diff --git a/src/core/hle/service/set/set.cpp b/src/core/hle/service/set/set.cpp index f761c2da4..4f1a8d6b7 100644 --- a/src/core/hle/service/set/set.cpp +++ b/src/core/hle/service/set/set.cpp @@ -83,7 +83,7 @@ void PushResponseLanguageCode(Kernel::HLERequestContext& ctx, std::size_t num_la } void GetAvailableLanguageCodesImpl(Kernel::HLERequestContext& ctx, std::size_t max_entries) { - const std::size_t requested_amount = ctx.GetWriteBufferSize() / sizeof(LanguageCode); + const std::size_t requested_amount = ctx.GetWriteBufferNumElements(); const std::size_t max_amount = std::min(requested_amount, max_entries); const std::size_t copy_amount = std::min(available_language_codes.size(), max_amount); const std::size_t copy_size = copy_amount * sizeof(LanguageCode);