From 566220278f6ae71f012572f554c6b0c2da140096 Mon Sep 17 00:00:00 2001
From: Steven Giesel <stgiesel35@gmail.com>
Date: Thu, 19 Oct 2023 20:10:53 +0200
Subject: [PATCH] Fixed Bug where Grow used a too small number

---
 CHANGELOG.md                                  |  4 ++++
 .../ValueStringBuilder.Append.cs              | 10 ++++++----
 .../ValueStringBuilder.cs                     |  2 +-
 .../AppendValueTypesBenchmark.cs              | 20 ++++++++++++++++++-
 .../ValueStringBuilder.Append.Tests.cs        | 17 ++++++++++++++++
 5 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a78954e..bbf1965 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/src/LinkDotNet.StringBuilder/ValueStringBuilder.Append.cs b/src/LinkDotNet.StringBuilder/ValueStringBuilder.Append.cs
index be75d17..d7a28ce 100644
--- a/src/LinkDotNet.StringBuilder/ValueStringBuilder.Append.cs
+++ b/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))
diff --git a/src/LinkDotNet.StringBuilder/ValueStringBuilder.cs b/src/LinkDotNet.StringBuilder/ValueStringBuilder.cs
index 3a90e38..665a7e1 100644
--- a/src/LinkDotNet.StringBuilder/ValueStringBuilder.cs
+++ b/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
 {
diff --git a/tests/LinkDotNet.StringBuilder.Benchmarks/AppendValueTypesBenchmark.cs b/tests/LinkDotNet.StringBuilder.Benchmarks/AppendValueTypesBenchmark.cs
index 6077f87..5e2a67b 100644
--- a/tests/LinkDotNet.StringBuilder.Benchmarks/AppendValueTypesBenchmark.cs
+++ b/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();
+    }
 }
\ No newline at end of file
diff --git a/tests/LinkDotNet.StringBuilder.UnitTests/ValueStringBuilder.Append.Tests.cs b/tests/LinkDotNet.StringBuilder.UnitTests/ValueStringBuilder.Append.Tests.cs
index 2e002a3..3677630 100644
--- a/tests/LinkDotNet.StringBuilder.UnitTests/ValueStringBuilder.Append.Tests.cs
+++ b/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();
+    }
 }
\ No newline at end of file