Hi! On Fri, Sep 11, 2020 at 1:29 PM Sergei Golubchik <s...@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 _______________________________________________ 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