Hi, Michael! This is a good catch!
But note that LONGLONG_BUFFER_SIZE already reserves +1 for '\0', it's defined as /* Max length needed for a buffer to hold a longlong or ulonglong + end \0 */ #define LONGLONG_BUFFER_SIZE 21 The problem is that String::set_int doesn't use LONGLONG_BUFFER_SIZE, it uses 20*cs->mbmaxlen+1 This +1 is supposed to reserve one byte for '\0'. Which is incorrect, because alloc() does it too, so a better fix would be to remove +1 from String::set_int(). On Sep 08, Michael Widenius wrote: > revision-id: 099596296ce (mariadb-10.5.2-272-g099596296ce) > parent(s): 2de0c74f94c > author: Michael Widenius <michael.widen...@gmail.com> > committer: Michael Widenius <michael.widen...@gmail.com> > timestamp: 2020-09-03 15:17:56 +0300 > message: > > Avoid mallocs when using LONGLONG_BUFFER_SIZE > > When using String::alloc() to allocate a string, the String ensures that > there is space for an extra NULL byte in the buffer and if not, reallocates > the string. This is a problem with the String::set_int() that calls > alloc(LONGLONG_BUFFER_SIZE), which forces an extra malloc/free to > happen. > > Fixed by using LONGLONG_BUFFER_SIZE+1 for StringBuffer's. > > diff --git a/sql/field.cc b/sql/field.cc > index ab7254e483d..2be4da8d4a8 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -1085,7 +1085,7 @@ Field_longstr::make_packed_sort_key_part(uchar *buff, > uchar* > Field_longstr::pack_sort_string(uchar *to, const SORT_FIELD_ATTR *sort_field) > { > - StringBuffer<LONGLONG_BUFFER_SIZE> buf; > + StringBuffer<LONGLONG_BUFFER_SIZE+1> buf; > val_str(&buf, &buf); > return to + sort_field->pack_sort_string(to, &buf, field_charset()); > } > diff --git a/sql/item.cc b/sql/item.cc > index 4a26cb6c140..0d947749e01 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -3612,7 +3612,7 @@ String *Item_int::val_str(String *str) > > void Item_int::print(String *str, enum_query_type query_type) > { > - StringBuffer<LONGLONG_BUFFER_SIZE> buf; > + StringBuffer<LONGLONG_BUFFER_SIZE+1> buf; > // my_charset_bin is good enough for numbers > buf.set_int(value, unsigned_flag, &my_charset_bin); > str->append(buf); > Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp