Fixed Bug where Grow used a too small number

Author: linkdotnet

CHANGELOG.md

@@ -6,6 +6,10 @@ All notable changes to **ValueStringBuilder** will be documented in this file. T
 
 ## [Unreleased]
 
+### Changed
+- Fixed a bug, where in `Append` overflows the internal buffer and throws an exception
+- Use better struct layout to be more cache friendly
+
 ## [1.18.4] - 2023-10-14
 
 ### Changed

src/LinkDotNet.StringBuilder/ValueStringBuilder.Append.cs

@@ -14,9 +14,10 @@ public void Append(bool value)
     {
         // 5 is the length of the string "False"
         // So we can check if we have enough space in the buffer
-        if (bufferPosition + 5 > buffer.Length)
+        var newSize = bufferPosition + 5;
+        if (newSize > buffer.Length)
         {
-            Grow(bufferPosition);
+            Grow(newSize);
         }
 
         if (!value.TryFormat(buffer[bufferPosition..], out var charsWritten))
@@ -105,9 +106,10 @@ public void AppendLine(scoped ReadOnlySpan<char> str)
     private void AppendSpanFormattable<T>(T value, ReadOnlySpan<char> format = default, int bufferSize = 36)
         where T : ISpanFormattable
     {
-        if (bufferSize + bufferPosition >= Capacity)
+        var newSize = bufferSize + bufferPosition;
+        if (newSize >= Capacity)
         {
-            Grow(bufferSize + bufferPosition);
+            Grow(newSize);
         }
 
         if (!value.TryFormat(buffer[bufferPosition..], out var written, format, null))

src/LinkDotNet.StringBuilder/ValueStringBuilder.cs

@@ -11,7 +11,7 @@ namespace LinkDotNet.StringBuilder;
 /// The <see cref="ValueStringBuilder"/> is declared as ref struct which brings certain limitations with it.
 /// You can only use it in another ref struct or as a local variable.
 /// </remarks>
-[StructLayout(LayoutKind.Auto)]
+[StructLayout(LayoutKind.Sequential)]
 [SkipLocalsInit]
 public ref partial struct ValueStringBuilder
 {

tests/LinkDotNet.StringBuilder.Benchmarks/AppendValueTypesBenchmark.cs

@@ -8,7 +8,7 @@ public class AppendValueTypes
     private const int NumberOfIterations = 25;
 
     [Benchmark]
-    public string DotNetStringBuilder()
+    public string DotNetStringBuilderAppendValue()
     {
         var builder = new System.Text.StringBuilder();
 
@@ -24,4 +24,22 @@ public string DotNetStringBuilder()
 
         return builder.ToString();
     }
+
+    [Benchmark]
+    public string ValueStringBuilderAppendValue()
+    {
+        using var builder = new ValueStringBuilder();
+
+        for (var i = 0; i < NumberOfIterations; i++)
+        {
+            builder.Append(true);
+            builder.Append(int.MaxValue);
+            builder.Append(decimal.MaxValue);
+            builder.Append(byte.MinValue);
+            builder.Append(float.Epsilon);
+            builder.Append(double.Epsilon);
+        }
+
+        return builder.ToString();
+    }
 }

tests/LinkDotNet.StringBuilder.UnitTests/ValueStringBuilder.Append.Tests.cs

@@ -179,4 +179,21 @@ public void GivenAStringWithWhitespace_WhenTrimIsCalled_ThenTheStringShouldBeTri
 
         builder.ToString().Should().Be("Hello World");
     }
+
+    [Fact]
+    public void GivenMultipleValues_WhenCallingAppend_NotCrashing()
+    {
+        using var builder = new ValueStringBuilder();
+        builder.Append(true);
+        builder.Append(false);
+        builder.Append(true);
+        builder.Append(false);
+        builder.Append(true);
+        builder.Append(false);
+        builder.Append(true);
+
+        builder.Append(false);
+
+        builder.ToString().Should().NotBeNull();
+    }
 }