From 2815c505c35097b861c6646d98eadb5ebbb8a22c Mon Sep 17 00:00:00 2001 From: Oliver Booth Date: Sat, 1 Apr 2023 17:11:05 +0100 Subject: [PATCH] fix: fix incorrect float/double being written for netstandard2.1 The call to _To_Bits yields the result containing the same bytes, but not the same value. This value was then stored as-is into the parameter, which causes a conversion on how the value is stored, ultimately causing the wrong value to be written to the stream. --- CHANGELOG.md | 2 + X10D.Tests/src/IO/StreamTests.cs | 104 +++++++++++++++++++++++-------- X10D/src/IO/StreamExtensions.cs | 40 +++++++----- 3 files changed, 106 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 667e6ef..0a59c9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 using the [GetOffsetAndLength](https://learn.microsoft.com/en-us/dotnet/api/system.range.getoffsetandlength?view=net-7.0) method. - X10D: `Stream.ReadSingle(Endianness)` now returns a float instead of a double. +- X10D: Fixed `Stream.ReadDouble(Endianness)`, `Stream.ReadSingle(Endianness)`, `Stream.WriteDouble(double, Endianness)`, + `Stream.WriteSingle(float, Endianness)` writing and reading the wrong endianness. ### Changed diff --git a/X10D.Tests/src/IO/StreamTests.cs b/X10D.Tests/src/IO/StreamTests.cs index 77809fd..47d4e0d 100644 --- a/X10D.Tests/src/IO/StreamTests.cs +++ b/X10D.Tests/src/IO/StreamTests.cs @@ -234,47 +234,99 @@ public class StreamTests } [TestMethod] - public void ReadDouble_WriteDouble_ShouldBeSymmetric() + public void WriteDouble_ShouldWriteBigEndian_GivenBigEndian() { using var stream = new MemoryStream(); - stream.Write(420.0, BitConverter.IsLittleEndian ? Endianness.LittleEndian : Endianness.BigEndian); - - stream.Position = 0; - Assert.AreEqual(420.0, stream.ReadDouble(), 1e-6); - - stream.Position = 0; - stream.Write(420.0, Endianness.LittleEndian); - - stream.Position = 0; - Assert.AreEqual(420.0, stream.ReadDouble(Endianness.LittleEndian), 1e-6); - - stream.Position = 0; stream.Write(420.0, Endianness.BigEndian); - + Assert.AreEqual(8, stream.Position); stream.Position = 0; - Assert.AreEqual(420.0, stream.ReadDouble(Endianness.BigEndian), 1e-6); + + Span actual = stackalloc byte[8]; + ReadOnlySpan expected = stackalloc byte[] {0x40, 0x7A, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00}; + int read = stream.Read(actual); + + byte[] actualArray = actual.ToArray(); + byte[] expectedArray = expected.ToArray(); + + string actualBytes = string.Join(", ", actualArray.Select(b => $"0x{b:X2}")); + string expectedBytes = string.Join(", ", expectedArray.Select(b => $"0x{b:X2}")); + Trace.WriteLine($"Actual bytes: {actualBytes}"); + Trace.WriteLine($"Expected bytes: {expectedBytes}"); + + Assert.AreEqual(8, read); + CollectionAssert.AreEqual(expectedArray, actualArray); } [TestMethod] - public void ReadDecimal_WriteSingle_ShouldBeSymmetric() + public void WriteDouble_ShouldWriteLittleEndian_GivenLittleEndian() { using var stream = new MemoryStream(); - stream.Write(420.0m, BitConverter.IsLittleEndian ? Endianness.LittleEndian : Endianness.BigEndian); - + stream.Write(420.0, Endianness.LittleEndian); + Assert.AreEqual(8, stream.Position); stream.Position = 0; - Assert.AreEqual(420.0m, stream.ReadDecimal()); - stream.Position = 0; - stream.Write(420.0m, Endianness.LittleEndian); + Span actual = stackalloc byte[8]; + ReadOnlySpan expected = stackalloc byte[] {0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x7A, 0x40}; + int read = stream.Read(actual); - stream.Position = 0; - Assert.AreEqual(420.0m, stream.ReadDecimal(Endianness.LittleEndian)); + byte[] actualArray = actual.ToArray(); + byte[] expectedArray = expected.ToArray(); - stream.Position = 0; - stream.Write(420.0m, Endianness.BigEndian); + string actualBytes = string.Join(", ", actualArray.Select(b => $"0x{b:X2}")); + string expectedBytes = string.Join(", ", expectedArray.Select(b => $"0x{b:X2}")); + Trace.WriteLine($"Actual bytes: {actualBytes}"); + Trace.WriteLine($"Expected bytes: {expectedBytes}"); + Assert.AreEqual(8, read); + CollectionAssert.AreEqual(expectedArray, actualArray); + } + + [TestMethod] + public void WriteSingle_ShouldWriteBigEndian_GivenBigEndian() + { + using var stream = new MemoryStream(); + stream.Write(420.0f, Endianness.BigEndian); + Assert.AreEqual(4, stream.Position); stream.Position = 0; - Assert.AreEqual(420.0m, stream.ReadDecimal(Endianness.BigEndian)); + + Span actual = stackalloc byte[4]; + ReadOnlySpan expected = stackalloc byte[] {0x43, 0xD2, 0x00, 0x00}; + int read = stream.Read(actual); + + byte[] actualArray = actual.ToArray(); + byte[] expectedArray = expected.ToArray(); + + string actualBytes = string.Join(", ", actualArray.Select(b => $"0x{b:X2}")); + string expectedBytes = string.Join(", ", expectedArray.Select(b => $"0x{b:X2}")); + Trace.WriteLine($"Actual bytes: {actualBytes}"); + Trace.WriteLine($"Expected bytes: {expectedBytes}"); + + Assert.AreEqual(4, read); + CollectionAssert.AreEqual(expectedArray, actualArray); + } + + [TestMethod] + public void WriteSingle_ShouldWriteLittleEndian_GivenLittleEndian() + { + using var stream = new MemoryStream(); + stream.Write(420.0f, Endianness.LittleEndian); + Assert.AreEqual(4, stream.Position); + stream.Position = 0; + + Span actual = stackalloc byte[4]; + ReadOnlySpan expected = stackalloc byte[] {0x00, 0x00, 0xD2, 0x43}; + int read = stream.Read(actual); + + byte[] actualArray = actual.ToArray(); + byte[] expectedArray = expected.ToArray(); + + string actualBytes = string.Join(", ", actualArray.Select(b => $"0x{b:X2}")); + string expectedBytes = string.Join(", ", expectedArray.Select(b => $"0x{b:X2}")); + Trace.WriteLine($"Actual bytes: {actualBytes}"); + Trace.WriteLine($"Expected bytes: {expectedBytes}"); + + Assert.AreEqual(4, read); + CollectionAssert.AreEqual(expectedArray, actualArray); } [TestMethod] diff --git a/X10D/src/IO/StreamExtensions.cs b/X10D/src/IO/StreamExtensions.cs index 17611b8..a843efd 100644 --- a/X10D/src/IO/StreamExtensions.cs +++ b/X10D/src/IO/StreamExtensions.cs @@ -996,27 +996,33 @@ public static class StreamExtensions if (endianness == Endianness.LittleEndian) { #if NET5_0_OR_GREATER - BinaryPrimitives.WriteSingleLittleEndian(buffer, value); + BinaryPrimitives.WriteDoubleLittleEndian(buffer, value); #else if (BitConverter.IsLittleEndian) { - value = BinaryPrimitives.ReverseEndianness(BitConverter.SingleToInt32Bits(value)); + MemoryMarshal.Write(buffer, ref value); + } + else + { + int temp = BinaryPrimitives.ReverseEndianness(BitConverter.SingleToInt32Bits(value)); + MemoryMarshal.Write(buffer, ref temp); } - - System.Runtime.InteropServices.MemoryMarshal.Write(buffer, ref value); #endif } else { #if NET5_0_OR_GREATER - BinaryPrimitives.WriteSingleBigEndian(buffer, value); + BinaryPrimitives.WriteDoubleBigEndian(buffer, value); #else if (BitConverter.IsLittleEndian) { - value = BinaryPrimitives.ReverseEndianness(BitConverter.SingleToInt32Bits(value)); + int temp = BinaryPrimitives.ReverseEndianness(BitConverter.SingleToInt32Bits(value)); + MemoryMarshal.Write(buffer, ref temp); + } + else + { + MemoryMarshal.Write(buffer, ref value); } - - System.Runtime.InteropServices.MemoryMarshal.Write(buffer, ref value); #endif } @@ -1064,10 +1070,13 @@ public static class StreamExtensions #else if (BitConverter.IsLittleEndian) { - value = BinaryPrimitives.ReverseEndianness(BitConverter.DoubleToInt64Bits(value)); + MemoryMarshal.Write(buffer, ref value); + } + else + { + long temp = BinaryPrimitives.ReverseEndianness(BitConverter.DoubleToInt64Bits(value)); + MemoryMarshal.Write(buffer, ref temp); } - - System.Runtime.InteropServices.MemoryMarshal.Write(buffer, ref value); #endif } else @@ -1077,10 +1086,13 @@ public static class StreamExtensions #else if (BitConverter.IsLittleEndian) { - value = BinaryPrimitives.ReverseEndianness(BitConverter.DoubleToInt64Bits(value)); + long temp = BinaryPrimitives.ReverseEndianness(BitConverter.DoubleToInt64Bits(value)); + MemoryMarshal.Write(buffer, ref temp); + } + else + { + MemoryMarshal.Write(buffer, ref value); } - - System.Runtime.InteropServices.MemoryMarshal.Write(buffer, ref value); #endif }