From edfd71d221201832cfe6f3714c41b518b4d58fdc Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 06:49:17 +0700 Subject: [PATCH 1/7] Simplify template and method parameters Move the `std::vector` restriction into an `enable_if` clause. Use member type definitions instead of template arguments. --- src/Stream/Writer.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index e9873a62..25a69e4e 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -37,13 +37,17 @@ namespace Stream WriteImplementation(&object, sizeof(object)); } - // Vector of trvially copyable data types - template + // Vector of trivially copyable data types + template inline - std::enable_if_t::value> - Write(const std::vector& vector) { + std::enable_if_t< + !std::is_trivially_copyable::value && + std::is_same>::value && + std::is_trivially_copyable::value + > + Write(const T& vector) { // Size calculation can't possibly overflow since the vector size necessarily fits in memory - WriteImplementation(vector.data(), vector.size() * sizeof(T)); + WriteImplementation(vector.data(), vector.size() * sizeof(typename T::value_type)); } // Size prefixed vector data types From 71f8161399c6b5837f435d524dee7931a012c3cd Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 06:54:48 +0700 Subject: [PATCH 2/7] 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/Writer.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index 25a69e4e..0705512c 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -37,17 +37,16 @@ namespace Stream WriteImplementation(&object, sizeof(object)); } - // Vector of trivially copyable data types + // Non-trivial contiguous container of trivially copyable data types template inline std::enable_if_t< !std::is_trivially_copyable::value && - std::is_same>::value && std::is_trivially_copyable::value > - Write(const T& vector) { - // Size calculation can't possibly overflow since the vector size necessarily fits in memory - WriteImplementation(vector.data(), vector.size() * sizeof(typename T::value_type)); + Write(const T& container) { + // Size calculation can't possibly overflow since the container size necessarily fits in memory + WriteImplementation(container.data(), container.size() * sizeof(typename T::value_type)); } // Size prefixed vector data types From cef23971c3caf9cb3b40cfdcc4d422790498ec32 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:01:39 +0700 Subject: [PATCH 3/7] Remove string write overload This is now served by the more general non-trivial contiguous container of trivial types case. --- src/Stream/Writer.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index 0705512c..10667f12 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -64,14 +64,6 @@ namespace Stream Write(vector); } - // String data types - // Does not write null terminator unless specifically included in string - template - void Write(const std::basic_string& string) { - // Size calculation can't possibly overflow since the string size necessarily fits in memory - Write(&string[0], string.size() * sizeof(CharT)); - } - // Size prefixed string data types // Ex: Write(string); // Write 32-bit string length followed by string data // Does not write null terminator unless specifically included in string From 7b79eb732915efe5afaca1ae217b36d1ecc06688 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:14:25 +0700 Subject: [PATCH 4/7] Simplify template and method parameters Move the `std::vector` restriction into an `enable_if` clause. Use member type definitions instead of template arguments. --- src/Stream/Writer.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index 10667f12..828d9238 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -51,8 +51,11 @@ namespace Stream // Size prefixed vector data types // Ex: Write(vector); // Write 32-bit vector size followed by vector data - template - void Write(const std::vector& vector) { + template + std::enable_if_t< + std::is_same>::value + > + Write(const T& vector) { auto vectorSize = vector.size(); // This check is trivially false if SizeType is larger than max vector size if (vectorSize > std::numeric_limits::max()) { From bf47864d309495f1b75ef4f222d7c3daedd7cd05 Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:16:37 +0700 Subject: [PATCH 5/7] Remove temporary local variable It doesn't really need to exist, and eliminating it makes the implementation match the size prefixed string write method. --- src/Stream/Writer.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index 828d9238..b36c26b6 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -61,9 +61,8 @@ namespace Stream if (vectorSize > std::numeric_limits::max()) { throw std::runtime_error("Vector too large to save size field"); } - // This can't overflow due to check above - auto typedSize = static_cast(vectorSize); - Write(typedSize); + // Cast can't overflow due to check above + Write(static_cast(vectorSize)); Write(vector); } From 0b96d3f8ef968712f476a5c6901b706d55cc551f Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:21:38 +0700 Subject: [PATCH 6/7] Generalize size prefixed write to any container This can potentially handle the size prefix part of any container write, including containers we don't yet support serialization for. Currently we simply delegate for the details of writing the actual container contents. --- src/Stream/Writer.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index b36c26b6..d662f986 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -49,21 +49,21 @@ namespace Stream WriteImplementation(container.data(), container.size() * sizeof(typename T::value_type)); } - // Size prefixed vector data types + // Size prefixed container data types // Ex: Write(vector); // Write 32-bit vector size followed by vector data template std::enable_if_t< - std::is_same>::value + true > - Write(const T& vector) { - auto vectorSize = vector.size(); - // This check is trivially false if SizeType is larger than max vector size - if (vectorSize > std::numeric_limits::max()) { - throw std::runtime_error("Vector too large to save size field"); + Write(const T& container) { + auto containerSize = container.size(); + // This check is trivially false if SizeType is larger than max container size + if (containerSize > std::numeric_limits::max()) { + throw std::runtime_error("Container too large to save size field"); } // Cast can't overflow due to check above - Write(static_cast(vectorSize)); - Write(vector); + Write(static_cast(containerSize)); + Write(container); } // Size prefixed string data types From 106e003b4f2a65dae65ce25360fa50ef5c0a6dcb Mon Sep 17 00:00:00 2001 From: Daniel Stevens Date: Mon, 25 Mar 2019 07:26:46 +0700 Subject: [PATCH 7/7] Remove string size prefixed write overload This is now served by the more general size prefixed container write case. --- src/Stream/Writer.h | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/Stream/Writer.h b/src/Stream/Writer.h index d662f986..b4c33d32 100644 --- a/src/Stream/Writer.h +++ b/src/Stream/Writer.h @@ -66,21 +66,6 @@ namespace Stream Write(container); } - // Size prefixed string data types - // Ex: Write(string); // Write 32-bit string length followed by string data - // Does not write null terminator unless specifically included in string - template - void Write(const std::basic_string& string) { - auto stringSize = string.size(); - // This check is trivially false if SizeType is larger than max string size - if (stringSize > std::numeric_limits::max()) { - throw std::runtime_error("String's size is too large to write in provided size field"); - } - // This can't overflow due to check above - Write(static_cast(stringSize)); - Write(string); - } - // Copy a Reader to a Writer static const std::size_t DefaultCopyChunkSize = 0x00020000; template