Hi! On Fri, Sep 11, 2020 at 1:29 PM Sergei Golubchik <serg@mariadb.org> wrote:
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().
This is part of the problem. However if String::set_int would use LONGLONG_BUFFER_SIZE for allocation (which it should instead of 20) we still would have a problem. I think it's right that any allocation for a String buffer should include space for the extra end null, as we are in may cases uses c_ptr() for these buffers. alloc() does reserve place for an extra \0. However the problem is that realloc tests if requested_buffer_length >= allocated_length and if not does an allocation. In effect this means that the buffer, with current code, needs to be 2 bigger than the 'size-for-string-data' The fix I intend to do later in in 10.6 is to change the >= to > in realloc() This will fix the problem and we can use LONGLONG_BUFFER_SIZE everywhere without having a second malloc. This will however require some other changes and testing that I don't have time to do now, so I prefer to keep things as it's now until the above code is done. Regards, Monty