From 9b75481755c8d566bc666465d659115bba2b2578 Mon Sep 17 00:00:00 2001 From: David <25727384+ogniK5377@users.noreply.github.com> Date: Mon, 3 Aug 2020 21:28:54 +1000 Subject: [PATCH] ipc: Allow all trivially copyable objects to be passed directly into WriteBuffer (#4465) * ipc: Allow all trivially copyable objects to be passed directly into WriteBuffer With the support of C++20, we can use concepts to deduce if a type is an STL container or not. * More agressive concept for stl containers * Add -fconcepts * Move to common namespace * Add Common::IsBaseOf --- src/CMakeLists.txt | 1 + src/common/CMakeLists.txt | 1 + src/common/concepts.h | 32 +++++++++++++++++++ src/core/hle/kernel/hle_ipc.h | 30 +++++++++-------- src/core/hle/service/acc/acc.cpp | 6 ++-- src/core/hle/service/audio/hwopus.cpp | 2 +- src/core/hle/service/bcat/module.cpp | 2 +- src/core/hle/service/es/es.cpp | 2 +- src/core/hle/service/nfp/nfp.cpp | 8 ++--- src/core/hle/service/set/set.cpp | 2 +- src/core/hle/service/time/time.cpp | 4 +-- .../hle/service/time/time_zone_service.cpp | 4 +-- 12 files changed, 64 insertions(+), 30 deletions(-) create mode 100644 src/common/concepts.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1e977e8a8c..54dca33024 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -60,6 +60,7 @@ else() -Wmissing-declarations -Wno-attributes -Wno-unused-parameter + -fconcepts ) if (ARCHITECTURE_x86_64) diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index d120c8d3d9..78c3bfb3b9 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -110,6 +110,7 @@ add_library(common STATIC common_funcs.h common_paths.h common_types.h + concepts.h dynamic_library.cpp dynamic_library.h fiber.cpp diff --git a/src/common/concepts.h b/src/common/concepts.h new file mode 100644 index 0000000000..db5fb373d9 --- /dev/null +++ b/src/common/concepts.h @@ -0,0 +1,32 @@ +// Copyright 2020 yuzu emulator team +// Licensed under GPLv2 or any later version +// Refer to the license.txt file included. + +#pragma once + +namespace Common { + +#include + +// Check if type is like an STL container +template +concept IsSTLContainer = requires(T t) { + typename T::value_type; + typename T::iterator; + typename T::const_iterator; + // TODO(ogniK): Replace below is std::same_as when MSVC supports it. + t.begin(); + t.end(); + t.cbegin(); + t.cend(); + t.data(); + t.size(); +}; + +// Check if type T is derived from T2 +template +concept IsBaseOf = requires { + std::is_base_of_v; +}; + +} // namespace Common diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index b316739285..f3277b766e 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -13,6 +13,7 @@ #include #include #include "common/common_types.h" +#include "common/concepts.h" #include "common/swap.h" #include "core/hle/ipc.h" #include "core/hle/kernel/object.h" @@ -193,23 +194,24 @@ public: /* Helper function to write a buffer using the appropriate buffer descriptor * - * @tparam ContiguousContainer an arbitrary container that satisfies the - * ContiguousContainer concept in the C++ standard library. + * @tparam T an arbitrary container that satisfies the + * ContiguousContainer concept in the C++ standard library or a trivially copyable type. * - * @param container The container to write the data of into a buffer. + * @param data The container/data to write into a buffer. * @param buffer_index The buffer in particular to write to. */ - template >> - std::size_t WriteBuffer(const ContiguousContainer& container, - std::size_t buffer_index = 0) const { - using ContiguousType = typename ContiguousContainer::value_type; - - static_assert(std::is_trivially_copyable_v, - "Container to WriteBuffer must contain trivially copyable objects"); - - return WriteBuffer(std::data(container), std::size(container) * sizeof(ContiguousType), - buffer_index); + template >> + std::size_t WriteBuffer(const T& data, std::size_t buffer_index = 0) const { + if constexpr (Common::IsSTLContainer) { + using ContiguousType = typename T::value_type; + static_assert(std::is_trivially_copyable_v, + "Container to WriteBuffer must contain trivially copyable objects"); + return WriteBuffer(std::data(data), std::size(data) * sizeof(ContiguousType), + buffer_index); + } else { + static_assert(std::is_trivially_copyable_v, "T must be trivially copyable"); + return WriteBuffer(&data, sizeof(T), buffer_index); + } } /// Helper function to get the size of the input buffer diff --git a/src/core/hle/service/acc/acc.cpp b/src/core/hle/service/acc/acc.cpp index 8ac856ec3a..63e4aeca00 100644 --- a/src/core/hle/service/acc/acc.cpp +++ b/src/core/hle/service/acc/acc.cpp @@ -286,9 +286,7 @@ protected: ProfileBase profile_base{}; ProfileData data{}; if (profile_manager.GetProfileBaseAndData(user_id, profile_base, data)) { - std::array raw_data; - std::memcpy(raw_data.data(), &data, sizeof(ProfileData)); - ctx.WriteBuffer(raw_data); + ctx.WriteBuffer(data); IPC::ResponseBuilder rb{ctx, 16}; rb.Push(RESULT_SUCCESS); rb.PushRaw(profile_base); @@ -333,7 +331,7 @@ protected: std::vector buffer(size); image.ReadBytes(buffer.data(), buffer.size()); - ctx.WriteBuffer(buffer.data(), buffer.size()); + ctx.WriteBuffer(buffer); rb.Push(size); } diff --git a/src/core/hle/service/audio/hwopus.cpp b/src/core/hle/service/audio/hwopus.cpp index d19513cbbd..f1d81602ce 100644 --- a/src/core/hle/service/audio/hwopus.cpp +++ b/src/core/hle/service/audio/hwopus.cpp @@ -92,7 +92,7 @@ private: if (performance) { rb.Push(*performance); } - ctx.WriteBuffer(samples.data(), samples.size() * sizeof(s16)); + ctx.WriteBuffer(samples); } bool DecodeOpusData(u32& consumed, u32& sample_count, const std::vector& input, diff --git a/src/core/hle/service/bcat/module.cpp b/src/core/hle/service/bcat/module.cpp index 603b64d4fa..db0e06ca1d 100644 --- a/src/core/hle/service/bcat/module.cpp +++ b/src/core/hle/service/bcat/module.cpp @@ -112,7 +112,7 @@ private: void GetImpl(Kernel::HLERequestContext& ctx) { LOG_DEBUG(Service_BCAT, "called"); - ctx.WriteBuffer(&impl, sizeof(DeliveryCacheProgressImpl)); + ctx.WriteBuffer(impl); IPC::ResponseBuilder rb{ctx, 2}; rb.Push(RESULT_SUCCESS); diff --git a/src/core/hle/service/es/es.cpp b/src/core/hle/service/es/es.cpp index a41c73c48e..c2737a365f 100644 --- a/src/core/hle/service/es/es.cpp +++ b/src/core/hle/service/es/es.cpp @@ -160,7 +160,7 @@ private: return; } - ctx.WriteBuffer(key.data(), key.size()); + ctx.WriteBuffer(key); IPC::ResponseBuilder rb{ctx, 2}; rb.Push(RESULT_SUCCESS); diff --git a/src/core/hle/service/nfp/nfp.cpp b/src/core/hle/service/nfp/nfp.cpp index 4b79eb81d6..5e2d769a4d 100644 --- a/src/core/hle/service/nfp/nfp.cpp +++ b/src/core/hle/service/nfp/nfp.cpp @@ -127,7 +127,7 @@ private: const u32 array_size = rp.Pop(); LOG_DEBUG(Service_NFP, "called, array_size={}", array_size); - ctx.WriteBuffer(&device_handle, sizeof(device_handle)); + ctx.WriteBuffer(device_handle); IPC::ResponseBuilder rb{ctx, 3}; rb.Push(RESULT_SUCCESS); @@ -220,7 +220,7 @@ private: tag_info.protocol = 1; // TODO(ogniK): Figure out actual values tag_info.tag_type = 2; - ctx.WriteBuffer(&tag_info, sizeof(TagInfo)); + ctx.WriteBuffer(tag_info); rb.Push(RESULT_SUCCESS); } @@ -237,7 +237,7 @@ private: IPC::ResponseBuilder rb{ctx, 2}; auto amiibo = nfp_interface.GetAmiiboBuffer(); - ctx.WriteBuffer(&amiibo.model_info, sizeof(amiibo.model_info)); + ctx.WriteBuffer(amiibo.model_info); rb.Push(RESULT_SUCCESS); } @@ -283,7 +283,7 @@ private: CommonInfo common_info{}; common_info.application_area_size = 0; - ctx.WriteBuffer(&common_info, sizeof(CommonInfo)); + ctx.WriteBuffer(common_info); IPC::ResponseBuilder rb{ctx, 2}; rb.Push(RESULT_SUCCESS); diff --git a/src/core/hle/service/set/set.cpp b/src/core/hle/service/set/set.cpp index 34fe2fd82b..e64777668e 100644 --- a/src/core/hle/service/set/set.cpp +++ b/src/core/hle/service/set/set.cpp @@ -106,7 +106,7 @@ void GetKeyCodeMapImpl(Kernel::HLERequestContext& ctx) { IPC::ResponseBuilder rb{ctx, 2}; rb.Push(RESULT_SUCCESS); - ctx.WriteBuffer(&layout, sizeof(KeyboardLayout)); + ctx.WriteBuffer(layout); } } // Anonymous namespace diff --git a/src/core/hle/service/time/time.cpp b/src/core/hle/service/time/time.cpp index 13e4b38189..ee4fa4b485 100644 --- a/src/core/hle/service/time/time.cpp +++ b/src/core/hle/service/time/time.cpp @@ -290,7 +290,7 @@ void Module::Interface::GetClockSnapshot(Kernel::HLERequestContext& ctx) { IPC::ResponseBuilder rb{ctx, 2}; rb.Push(RESULT_SUCCESS); - ctx.WriteBuffer(&clock_snapshot, sizeof(Clock::ClockSnapshot)); + ctx.WriteBuffer(clock_snapshot); } void Module::Interface::GetClockSnapshotFromSystemClockContext(Kernel::HLERequestContext& ctx) { @@ -313,7 +313,7 @@ void Module::Interface::GetClockSnapshotFromSystemClockContext(Kernel::HLEReques IPC::ResponseBuilder rb{ctx, 2}; rb.Push(RESULT_SUCCESS); - ctx.WriteBuffer(&clock_snapshot, sizeof(Clock::ClockSnapshot)); + ctx.WriteBuffer(clock_snapshot); } void Module::Interface::CalculateStandardUserSystemClockDifferenceByUser( diff --git a/src/core/hle/service/time/time_zone_service.cpp b/src/core/hle/service/time/time_zone_service.cpp index db57ae069c..ff3a10b3e6 100644 --- a/src/core/hle/service/time/time_zone_service.cpp +++ b/src/core/hle/service/time/time_zone_service.cpp @@ -142,7 +142,7 @@ void ITimeZoneService::ToPosixTime(Kernel::HLERequestContext& ctx) { IPC::ResponseBuilder rb{ctx, 3}; rb.Push(RESULT_SUCCESS); rb.PushRaw(1); // Number of times we're returning - ctx.WriteBuffer(&posix_time, sizeof(s64)); + ctx.WriteBuffer(posix_time); } void ITimeZoneService::ToPosixTimeWithMyRule(Kernel::HLERequestContext& ctx) { @@ -164,7 +164,7 @@ void ITimeZoneService::ToPosixTimeWithMyRule(Kernel::HLERequestContext& ctx) { IPC::ResponseBuilder rb{ctx, 3}; rb.Push(RESULT_SUCCESS); rb.PushRaw(1); // Number of times we're returning - ctx.WriteBuffer(&posix_time, sizeof(s64)); + ctx.WriteBuffer(posix_time); } } // namespace Service::Time