Correct span and non-span versions of ToArray() and ToArrayPadded() methods (#8734)

* Correction of bug inside ToArray<T> methods

Avoid allocating too large buffers (len is expressed in bytes, not in Ts).
Added validation to ensure len is a multiple of SizeOf<T>() before converting to array.

* Update ByteBuffer.cs

* Refactor ToArray and ToArrayPadded methods

I understand from failed test that pos, len, padLeft and padRight are expressed in Ts

* Refactor ToArray and ToArrayPadded methods

* Final correction
All functions parameters expressed in bytes for homogeneity
Tests run:
  - UNSAFE_BYTEBUFFER=true/ENABLE_SPAN_T=true: passed
  - UNSAFE_BYTEBUFFER=true/ENABLE_SPAN_T=false: passed
  - UNSAFE_BYTEBUFFER=false/ENABLE_SPAN_T=false: passed
  - UNSAFE_BYTEBUFFER=false/ENABLE_SPAN_T=true: configuration forbidden by compilation
Correction of FlatBuffers.Test.csproj to allow UNSAFE_BYTEBUFFER/ENABLE_SPAN_T tests
Correction of FlatBuffersExampleTests.cs: I think the test was not run because it could not pass (to be reviewed carefully)

Author: vsmcea

net/FlatBuffers/ByteBuffer.cs

@@ -38,13 +38,13 @@
 
 using System;
 using System.Collections.Generic;
-using System.IO;
-using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Text;
 
 #if ENABLE_SPAN_T && UNSAFE_BYTEBUFFER
 using System.Buffers.Binary;
+#else
+using System.IO;
 #endif
 
 #if ENABLE_SPAN_T && !UNSAFE_BYTEBUFFER
@@ -245,34 +245,36 @@ public static int ArraySize<T>(Span<T> x)
 #endif
 
         // Get a portion of the buffer casted into an array of type T, given
-        // the buffer position and length.
-#if ENABLE_SPAN_T && UNSAFE_BYTEBUFFER
-        public T[] ToArray<T>(int pos, int len)
+        // the buffer position (in bytes) and length (in bytes).
+        public T[] ToArray<T>(int posInBytes, int lenInBytes)
             where T : struct
         {
-            AssertOffsetAndLength(pos, len);
-            return MemoryMarshal.Cast<byte, T>(_buffer.ReadOnlySpan.Slice(pos)).Slice(0, len).ToArray();
-        }
+            AssertOffsetAndLength(posInBytes, lenInBytes);
+#if ENABLE_SPAN_T && UNSAFE_BYTEBUFFER
+            return MemoryMarshal.Cast<byte, T>(_buffer.ReadOnlySpan.Slice(posInBytes, lenInBytes)).ToArray();
 #else
-        public T[] ToArray<T>(int pos, int len)
-            where T : struct
-        {
-            AssertOffsetAndLength(pos, len);
-            T[] arr = new T[len];
-            Buffer.BlockCopy(_buffer.Buffer, pos, arr, 0, ArraySize(arr));
-            return arr;
-        }
+            var lenInTs = ConvertBytesToTs<T>(lenInBytes);
+            var arrayOfTs = new T[lenInTs];
+            Buffer.BlockCopy(_buffer.Buffer, posInBytes, arrayOfTs, 0, lenInBytes);
+            return arrayOfTs;
 #endif
+        }
 
-        public T[] ToArrayPadded<T>(int pos, int len, int padLeft, int padRight)
+   			public T[] ToArrayPadded<T>(int posInBytes, int lenInBytes, int padLeftInBytes, int padRightInBytes)
             where T : struct
         {
-            AssertOffsetAndLength(pos, len);
-            int totalBytes = padLeft + len + padRight;
-            byte[] raw = _buffer.Buffer;
-            T[] arr = new T[totalBytes];
-            Buffer.BlockCopy(raw, pos, arr, padLeft, len);
-            return arr;
+            AssertOffsetAndLength(posInBytes, lenInBytes);
+            var padLeftInTs = ConvertBytesToTs<T>(padLeftInBytes);
+            var lenInTs = ConvertBytesToTs<T>(lenInBytes);
+            var padRightInTs = ConvertBytesToTs<T>(padRightInBytes);
+            var sizeInTs = padLeftInTs + lenInTs + padRightInTs;
+            var arrayOfTs = new T[sizeInTs];
+#if ENABLE_SPAN_T && UNSAFE_BYTEBUFFER
+            MemoryMarshal.Cast<byte, T>(_buffer.ReadOnlySpan.Slice(posInBytes, lenInBytes)).CopyTo(arrayOfTs.AsSpan().Slice(padLeftInTs));
+#else
+            Buffer.BlockCopy(_buffer.Buffer, posInBytes, arrayOfTs, padLeftInBytes, lenInBytes);
+#endif
+            return arrayOfTs;
         }
 
         public byte[] ToSizedArrayPadded(int padLeft, int padRight)
@@ -455,9 +457,31 @@ private void AssertOffsetAndLength(int offset, int length)
 #endif
         }
 
+        public static int ConvertTsToBytes<T>(int valueInTs)
+            where T : struct
+        {
+            var sizeOfT = SizeOf<T>();
+            var valueInBytes = valueInTs * sizeOfT;
+            return valueInBytes;
+        }
+
+        public static int ConvertBytesToTs<T>(int valueInBytes)
+            where T : struct
+        {
+            var sizeOfT = SizeOf<T>();
+            var valueInTs = valueInBytes / sizeOfT;
+#if !BYTEBUFFER_NO_BOUNDS_CHECK
+            if (valueInTs * sizeOfT != valueInBytes)
+            {
+                throw new ArgumentException($"{valueInBytes} must be a multiple of SizeOf<{typeof(T).Name}>()={sizeOfT}");
+            }
+#endif
+            return valueInTs;
+        }
+
 #if ENABLE_SPAN_T && UNSAFE_BYTEBUFFER
 
-        public void PutSbyte(int offset, sbyte value)
+    public void PutSbyte(int offset, sbyte value)
         {
             AssertOffsetAndLength(offset, sizeof(sbyte));
             _buffer.Span[offset] = (byte)value;

tests/FlatBuffers.Test/ByteBufferTests.cs

@@ -15,6 +15,7 @@
  */
 
 using System;
+using System.Linq;
 using System.Runtime.InteropServices;
 
 namespace Google.FlatBuffers.Test
@@ -363,34 +364,32 @@ public void ByteBuffer_To_Array_Float()
             fData[7] = 15.9994F;
             fData[8] = 18.9984F;
 
+            var sizeInBytes = ByteBuffer.ConvertTsToBytes<float>(fData.Length);
+
             // Tranfer it to a byte array
-            var buffer = new byte[sizeof(float) * fData.Length];
+            var buffer = new byte[sizeInBytes];
             Buffer.BlockCopy(fData, 0, buffer, 0, buffer.Length);
 
             // Create the Byte Buffer from byte array
             var uut = new ByteBuffer(buffer);
 
             // Get the full array back out and ensure they are equivalent
-            var bbArray = uut.ToArray<float>(0, len);
+            var bbArray = uut.ToArray<float>(0, sizeInBytes);
             Assert.ArrayEqual(fData, bbArray);
 
             // Get a portion of the full array back out and ensure the
             // subrange agrees
-            var bbArray2 = uut.ToArray<float>(4, len - 1);
-            Assert.AreEqual(bbArray2.Length, len - 1);
-            for (int i = 1; i < len - 1; i++)
-            {
-                Assert.AreEqual(fData[i], bbArray2[i - 1]);
-            }
+            var posInFloats = 4;
+            var lenInFloats = Math.Min(fData.Length - posInFloats - 1, 4);
+            var bbArray2 = uut.ToArray<float>(ByteBuffer.ConvertTsToBytes<float>(posInFloats), ByteBuffer.ConvertTsToBytes<float>(lenInFloats));
+            Assert.ArrayEqual(fData.Skip(posInFloats).Take(bbArray2.Length).ToArray(), bbArray2);
 
             // Get a sub portion of the full array back out and ensure the
             // subrange agrees
-            var bbArray3 = uut.ToArray<float>(8, len - 4);
-            Assert.AreEqual(bbArray3.Length, len - 4);
-            for (int i = 2; i < len - 4; i++)
-            {
-                Assert.AreEqual(fData[i], bbArray3[i - 2]);
-            }
+            posInFloats = 6;
+            lenInFloats = Math.Min(fData.Length - posInFloats - 1, 1);
+            var bbArray3 = uut.ToArray<float>(ByteBuffer.ConvertTsToBytes<float>(posInFloats), ByteBuffer.ConvertTsToBytes<float>(lenInFloats));
+            Assert.ArrayEqual(fData.Skip(posInFloats).Take(bbArray3.Length).ToArray(), bbArray3);
         }
 
         public void ByteBuffer_Put_Array_Helper<T>(T[] data, int typeSize)
@@ -405,7 +404,7 @@ public void ByteBuffer_Put_Array_Helper<T>(T[] data, int typeSize)
             Assert.AreEqual(1024 - typeSize * data.Length, nOffset);
 
             // Get the full array back out and ensure they are equivalent
-            var bbArray = uut.ToArray<T>(nOffset, data.Length);
+            var bbArray = uut.ToArray<T>(nOffset, ByteBuffer.ConvertTsToBytes<T>(data.Length));
             Assert.ArrayEqual(data, bbArray);
         }
 
@@ -421,7 +420,7 @@ public void ByteBuffer_Put_ArraySegment_Helper<T>(ArraySegment<T> data, int type
             Assert.AreEqual(1024 - typeSize * data.Count, nOffset);
 
             // Get the full array back out and ensure they are equivalent
-            var bbArray = uut.ToArray<T>(nOffset, data.Count);
+            var bbArray = uut.ToArray<T>(nOffset, ByteBuffer.ConvertTsToBytes<T>(data.Count));
             Assert.ArrayEqual(data, bbArray);
         }
 
@@ -443,7 +442,7 @@ public unsafe void ByteBuffer_Put_IntPtr_Helper<T>(T[] data, int typeSize)
                 Assert.AreEqual(1024 - sizeInBytes, nOffset);
 
                 // Get the full array back out and ensure they are equivalent
-                var bbArray = uut.ToArray<T>(nOffset, data.Length);
+                var bbArray = uut.ToArray<T>(nOffset, ByteBuffer.ConvertTsToBytes<T>(data.Length));
                 Assert.ArrayEqual(data, bbArray);
             }
             finally

tests/FlatBuffers.Test/FlatBuffers.Test.csproj

@@ -9,12 +9,12 @@
     <Compile Remove="Properties\AssemblyInfo.cs" />
   </ItemGroup>
 
-  <PropertyGroup Condition="'$(UnsafeByteBuffer)' == 'true'">
+  <PropertyGroup Condition="'$(UNSAFE_BYTEBUFFER)' == 'true'">
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <DefineConstants>$(DefineConstants);UNSAFE_BYTEBUFFER</DefineConstants>
   </PropertyGroup>
 
-  <PropertyGroup Condition="'$(EnableSpanT)' == 'true'">
+  <PropertyGroup Condition="'$(ENABLE_SPAN_T)' == 'true'">
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <DefineConstants>$(DefineConstants);ENABLE_SPAN_T</DefineConstants>
   </PropertyGroup>

tests/FlatBuffers.Test/FlatBuffersExampleTests.cs

@@ -268,10 +268,10 @@ private void TestBuffer(ByteBuffer bb)
             }
 
             var longArrayBytes = monster.GetVectorOfLongsBytes();
-            Assert.IsTrue(monster.VectorOfLongsLength * 8 == longArrayBytes.Length);
+            Assert.IsTrue(monster.VectorOfLongsLength == longArrayBytes.Length);
 
             var doubleArrayBytes = monster.GetVectorOfDoublesBytes();
-            Assert.IsTrue(monster.VectorOfDoublesLength * 8 == doubleArrayBytes.Length);
+            Assert.IsTrue(monster.VectorOfDoublesLength == doubleArrayBytes.Length);
             #else
             var nameBytes = monster.GetNameBytes().Value;
             Assert.AreEqual("MyMonster", Encoding.UTF8.GetString(nameBytes.Array, nameBytes.Offset, nameBytes.Count));