String.new(capacity:) don't substract termlen

byroot · byroot · commit 83f57ca3d225 · 2024-06-19T15:11:07.000+02:00
[Bug #20585] This was changed in 36a06ef because `String.new(1024)` would end up allocating `1025` bytes, but the problem with this change is that the caller may be trying to right size a String. So instead, we should just better document the behavior of `capacity:`.
diff --git a/doc/string/new.rdoc b/doc/string/new.rdoc
@@ -46,6 +46,10 @@ which may in turn affect performance:
   String.new(capacity: 1)
   String.new('foo', capacity: 4096)
 
+Note that Ruby strings are null-terminated internally, so the internal
+buffer size will be one or more bytes larger than the requested capacity
+depending on the encoding.
+
 The +string+, +encoding+, and +capacity+ arguments may all be used together:
 
   String.new('hello', encoding: 'UTF-8', capacity: 25)
diff --git a/string.c b/string.c
@@ -2055,12 +2055,7 @@ rb_str_s_new(int argc, VALUE *argv, VALUE klass)
         }
     }
 
-    long fake_len = capa - termlen;
-    if (fake_len < 0) {
-        fake_len = 0;
-    }
-
-    VALUE str = str_new0(klass, NULL, fake_len, termlen);
+    VALUE str = str_new0(klass, NULL, capa, termlen);
     STR_SET_LEN(str, 0);
     TERM_FILL(RSTRING_PTR(str), termlen);
 
diff --git a/test/-ext-/string/test_capacity.rb b/test/-ext-/string/test_capacity.rb
@@ -23,7 +23,7 @@ def test_capacity_normal
   def test_s_new_capacity
     assert_equal("", String.new(capacity: 1000))
     assert_equal(String, String.new(capacity: 1000).class)
-    assert_equal(10_000 - 1, capa(String.new(capacity: 10_000))) # Real capa doesn't account for termlen
+    assert_equal(10_000, capa(String.new(capacity: 10_000)))
 
     assert_equal("", String.new(capacity: -1000))
     assert_equal(capa(String.new(capacity: -10000)), capa(String.new(capacity: -1000)))

Original file line number	Diff line number	Diff line change
`@@ -2055,12 +2055,7 @@ rb_str_s_new(int argc, VALUE *argv, VALUE klass)`
`2055`	`2055`	`}`
`2056`	`2056`	`}`
`2057`	`2057`
`2058`		`- long fake_len = capa - termlen;`
`2059`		`- if (fake_len < 0) {`
`2060`		`- fake_len = 0;`
`2061`		`- }`
`2062`		`-`
`2063`		`- VALUE str = str_new0(klass, NULL, fake_len, termlen);`
	`2058`	`+ VALUE str = str_new0(klass, NULL, capa, termlen);`
`2064`	`2059`	`STR_SET_LEN(str, 0);`
`2065`	`2060`	`TERM_FILL(RSTRING_PTR(str), termlen);`
`2066`	`2061`