From 83f57ca3d225ce06abbc5eef6aec37de4fa36d58 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 19 Jun 2024 14:04:30 +0200 Subject: [PATCH] String.new(capacity:) don't substract termlen [Bug #20585] This was changed in 36a06efdd9f0604093dccbaf96d4e2cb17874dc8 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:`. --- doc/string/new.rdoc | 4 ++++ string.c | 7 +------ test/-ext-/string/test_capacity.rb | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/doc/string/new.rdoc b/doc/string/new.rdoc index d955e61c87..1d44291f76 100644 --- 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 index 650d09186a..c97351c0d3 100644 --- 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 index 2c6c51fdda..bcca64d85a 100644 --- a/test/-ext-/string/test_capacity.rb +++ b/test/-ext-/string/test_capacity.rb @@ -23,7 +23,7 @@ class Test_StringCapacity < Test::Unit::TestCase 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)))