From 344a0c91f27a848d6bd21f9a166bc044054331ff Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 23 Jul 2018 14:48:53 -0400 Subject: [PATCH 1/2] vi: std::move std::vector in constructors where applicable Allows avoiding unnecessary copies of the vector depending on the calling code. While we're at it, remove a redundant no-parameter base constructor call --- src/core/hle/service/vi/vi.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 3a69b85f9..049957503 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "common/alignment.h" #include "common/math_util.h" @@ -176,7 +177,7 @@ private: class IGBPConnectRequestParcel : public Parcel { public: - explicit IGBPConnectRequestParcel(const std::vector& buffer) : Parcel(buffer) { + explicit IGBPConnectRequestParcel(std::vector buffer) : Parcel(std::move(buffer)) { Deserialize(); } ~IGBPConnectRequestParcel() override = default; @@ -223,8 +224,8 @@ private: class IGBPSetPreallocatedBufferRequestParcel : public Parcel { public: - explicit IGBPSetPreallocatedBufferRequestParcel(const std::vector& buffer) - : Parcel(buffer) { + explicit IGBPSetPreallocatedBufferRequestParcel(std::vector buffer) + : Parcel(std::move(buffer)) { Deserialize(); } ~IGBPSetPreallocatedBufferRequestParcel() override = default; @@ -256,7 +257,7 @@ protected: class IGBPDequeueBufferRequestParcel : public Parcel { public: - explicit IGBPDequeueBufferRequestParcel(const std::vector& buffer) : Parcel(buffer) { + explicit IGBPDequeueBufferRequestParcel(std::vector buffer) : Parcel(std::move(buffer)) { Deserialize(); } ~IGBPDequeueBufferRequestParcel() override = default; @@ -307,7 +308,7 @@ protected: class IGBPRequestBufferRequestParcel : public Parcel { public: - explicit IGBPRequestBufferRequestParcel(const std::vector& buffer) : Parcel(buffer) { + explicit IGBPRequestBufferRequestParcel(std::vector buffer) : Parcel(std::move(buffer)) { Deserialize(); } ~IGBPRequestBufferRequestParcel() override = default; @@ -322,8 +323,7 @@ public: class IGBPRequestBufferResponseParcel : public Parcel { public: - explicit IGBPRequestBufferResponseParcel(NVFlinger::IGBPBuffer buffer) - : Parcel(), buffer(buffer) {} + explicit IGBPRequestBufferResponseParcel(NVFlinger::IGBPBuffer buffer) : buffer(buffer) {} ~IGBPRequestBufferResponseParcel() override = default; protected: @@ -340,7 +340,7 @@ protected: class IGBPQueueBufferRequestParcel : public Parcel { public: - explicit IGBPQueueBufferRequestParcel(const std::vector& buffer) : Parcel(buffer) { + explicit IGBPQueueBufferRequestParcel(std::vector buffer) : Parcel(std::move(buffer)) { Deserialize(); } ~IGBPQueueBufferRequestParcel() override = default; @@ -409,7 +409,7 @@ private: class IGBPQueryRequestParcel : public Parcel { public: - explicit IGBPQueryRequestParcel(const std::vector& buffer) : Parcel(buffer) { + explicit IGBPQueryRequestParcel(std::vector buffer) : Parcel(std::move(buffer)) { Deserialize(); } ~IGBPQueryRequestParcel() override = default; From 1432912ae88e750ae0aae8972d7e260538d271df Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 23 Jul 2018 14:52:32 -0400 Subject: [PATCH 2/2] vi: Add std::is_trivially_copyable checks to Read and Write functions It's undefined behavior to memcpy an object that isn't considered trivially copyable, so put a compile-time check in to make sure this doesn't occur. --- src/core/hle/service/vi/vi.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 049957503..993f1e65a 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include "common/alignment.h" @@ -44,7 +45,9 @@ public: template T Read() { + static_assert(std::is_trivially_copyable_v, "T must be trivially copyable."); ASSERT(read_index + sizeof(T) <= buffer.size()); + T val; std::memcpy(&val, buffer.data() + read_index, sizeof(T)); read_index += sizeof(T); @@ -54,7 +57,9 @@ public: template T ReadUnaligned() { + static_assert(std::is_trivially_copyable_v, "T must be trivially copyable."); ASSERT(read_index + sizeof(T) <= buffer.size()); + T val; std::memcpy(&val, buffer.data() + read_index, sizeof(T)); read_index += sizeof(T); @@ -88,8 +93,12 @@ public: template void Write(const T& val) { - if (buffer.size() < write_index + sizeof(T)) + static_assert(std::is_trivially_copyable_v, "T must be trivially copyable."); + + if (buffer.size() < write_index + sizeof(T)) { buffer.resize(buffer.size() + sizeof(T) + DefaultBufferSize); + } + std::memcpy(buffer.data() + write_index, &val, sizeof(T)); write_index += sizeof(T); write_index = Common::AlignUp(write_index, 4); @@ -97,7 +106,9 @@ public: template void WriteObject(const T& val) { - u32_le size = static_cast(sizeof(val)); + static_assert(std::is_trivially_copyable_v, "T must be trivially copyable."); + + const u32_le size = static_cast(sizeof(val)); Write(size); // TODO(Subv): Support file descriptors. Write(0); // Fd count.