From 663716d2d0b4c38f58b18fd145fb22d8e0c84231 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:47:50 +0700 Subject: [PATCH 1/6] Update code formatting for consistency Matches style used for stream write code. --- src/Stream/Reader.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Stream/Reader.h b/src/Stream/Reader.h index b1f272d6..8f964899 100644 --- a/src/Stream/Reader.h +++ b/src/Stream/Reader.h @@ -34,7 +34,9 @@ namespace Stream // Trivially copyable data types template - inline std::enable_if_t::value> Read(T& object) { + inline + std::enable_if_t::value> + Read(T& object) { ReadImplementation(&object, sizeof(object)); } @@ -42,7 +44,8 @@ namespace Stream // Reads into entire length of passed vector. Call vector.resize(vectorSize) before // passing vector into this function to ensure proper vector size is read template - inline void Read(std::vector& vector) { + inline + void Read(std::vector& vector) { // Size calculation can't possibly overflow since the vector size necessarily fits in memory ReadImplementation(vector.data(), vector.size() * sizeof(T)); } From 19dbfd239dc2a6fec18689bdc547d3bc1f302e19 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:55:35 +0700 Subject: [PATCH 2/6] Generalize to any contiguous container of trivial types Guard against the container itself being trivial to avoid ambiguous calls. This excludes `std::array`, which is served by the other method taking trivial types. A trivial container of trivial types is itself trivial. Note: Technically, this should guard for types with `.data()` and `.size()` members. Such members imply a contiguous container. --- src/Stream/Reader.h | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Stream/Reader.h b/src/Stream/Reader.h index 8f964899..a1783762 100644 --- a/src/Stream/Reader.h +++ b/src/Stream/Reader.h @@ -40,14 +40,18 @@ namespace Stream ReadImplementation(&object, sizeof(object)); } - // Vector data types - // Reads into entire length of passed vector. Call vector.resize(vectorSize) before - // passing vector into this function to ensure proper vector size is read - template + // Non-trivial contiguous container of trivially copyable data types + // Reads into entire length of passed container. Call container.resize(size) before + // passing container into this function to ensure proper container size is read + template inline - void Read(std::vector& vector) { - // Size calculation can't possibly overflow since the vector size necessarily fits in memory - ReadImplementation(vector.data(), vector.size() * sizeof(T)); + std::enable_if_t< + !std::is_trivially_copyable::value && + std::is_trivially_copyable::value + > + Read(T& container) { + // Size calculation can't possibly overflow since the container size necessarily fits in memory + ReadImplementation(container.data(), container.size() * sizeof(typename T::value_type)); } // Size prefixed vector data types From e2a3d6fb3c5808b0dde92541c90db6fa89770d31 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 08:12:23 +0700 Subject: [PATCH 3/6] Move string read method next to container read method These methods are basically identical, and may be combined into a single method at some point. --- src/Stream/Reader.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Stream/Reader.h b/src/Stream/Reader.h index a1783762..534961a9 100644 --- a/src/Stream/Reader.h +++ b/src/Stream/Reader.h @@ -54,6 +54,15 @@ namespace Stream ReadImplementation(container.data(), container.size() * sizeof(typename T::value_type)); } + // String data types + // Reads into entire length of passed string. Call string.resize(stringSize) before + // passing string into this function to ensure proper string size is read + template + void Read(std::basic_string& string) { + // Size calculation can't possibly overflow since the string size necessarily fits in memory + Read(&string[0], string.size() * sizeof(CharT)); + } + // Size prefixed vector data types // Ex: Read(vector); // Read 32-bit vector size, allocate space, then read vector data template @@ -73,15 +82,6 @@ namespace Stream Read(vector); } - // String data types - // Reads into entire length of passed string. Call string.resize(stringSize) before - // passing string into this function to ensure proper string size is read - template - void Read(std::basic_string& string) { - // Size calculation can't possibly overflow since the string size necessarily fits in memory - Read(&string[0], string.size() * sizeof(CharT)); - } - // Size prefixed string data types // Ex: Read(string); // Read 32-bit string length, allocate space, then read string data template From e9f0de9616268e9481254a73f3c1861fce2067b2 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 08:14:56 +0700 Subject: [PATCH 4/6] Add note about why string read method still exists It could potentially be collapsed with the method above, but needs C++17 flags set to allow that. Reference: https://en.cppreference.com/w/cpp/string/basic_string/data --- src/Stream/Reader.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Stream/Reader.h b/src/Stream/Reader.h index 534961a9..3881cb8f 100644 --- a/src/Stream/Reader.h +++ b/src/Stream/Reader.h @@ -57,6 +57,7 @@ namespace Stream // String data types // Reads into entire length of passed string. Call string.resize(stringSize) before // passing string into this function to ensure proper string size is read + // Note: C++17 added a non-const overload of `string.data()`, needed to collapse this with the above template void Read(std::basic_string& string) { // Size calculation can't possibly overflow since the string size necessarily fits in memory From e82f9cabf8bdbd0e20ad053cd3fb3caa741b1754 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 08:20:49 +0700 Subject: [PATCH 5/6] Generalize size prefixed read to any container This can potentially handle the size prefix part of any container read, including containers we don't yet support serialization for. Currently we simply delegate for the details of reading the actual container contents. --- src/Stream/Reader.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Stream/Reader.h b/src/Stream/Reader.h index 3881cb8f..5172ec3b 100644 --- a/src/Stream/Reader.h +++ b/src/Stream/Reader.h @@ -64,23 +64,23 @@ namespace Stream Read(&string[0], string.size() * sizeof(CharT)); } - // Size prefixed vector data types + // Size prefixed container data types // Ex: Read(vector); // Read 32-bit vector size, allocate space, then read vector data - template - void Read(std::vector& vector) { - SizeType vectorSize; - Read(vectorSize); + template + void Read(T& container) { + SizeType containerSize; + Read(containerSize); // This check is trivially false for unsigned SizeType - if (vectorSize < 0) { - throw std::runtime_error("Vector's size may not be a negative number"); + if (containerSize < 0) { + throw std::runtime_error("Container's size may not be a negative number"); } - // This check may be trivially false when SizeType is much smaller than max vector size - if (vectorSize > vector.max_size()) { - throw std::runtime_error("Vector's size is too big to fit in memory"); + // This check may be trivially false when SizeType is much smaller than max container size + if (containerSize > container.max_size()) { + throw std::runtime_error("Container's size is too big to fit in memory"); } - vector.clear(); - vector.resize(vectorSize); - Read(vector); + container.clear(); + container.resize(containerSize); + Read(container); } // Size prefixed string data types From 95ae9a6f1af3b59d94d5f37b27d4e68507baba6e Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 08:23:56 +0700 Subject: [PATCH 6/6] Remove string size prefixed read overload This is now served by the more general size prefixed container read case. --- src/Stream/Reader.h | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/Stream/Reader.h b/src/Stream/Reader.h index 5172ec3b..dd7a5d63 100644 --- a/src/Stream/Reader.h +++ b/src/Stream/Reader.h @@ -82,25 +82,5 @@ namespace Stream container.resize(containerSize); Read(container); } - - // Size prefixed string data types - // Ex: Read(string); // Read 32-bit string length, allocate space, then read string data - template - void Read(std::basic_string& string) { - SizeType stringSize; - Read(stringSize); - // This check is trivially false for unsigned SizeType - if (stringSize < 0) { - throw std::runtime_error("String's size may not be a negative number"); - } - // This check may be trivially false when SizeType is too small to overflow string size for CharT types - if (stringSize > string.max_size()) { - throw std::runtime_error("String's size is too big to fit in memory"); - } - - string.clear(); - string.resize(stringSize); - Read(string); - } }; }