maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Fri, Sep 11, 2020 at 1:29 PM Sergei Golubchik <serg@xxxxxxxxxxx> 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