Hi! Sorry for the delayed reply (but better late than never).
>>>>> "Kristian" == Kristian Nielsen <[email protected]> writes: Kristian> Hi Monty, Kristian> While fixing test failures in the MariaDB 5.5 tree, I encountered problems in Kristian> sql_string, related to changes you did there in 5.2. I managed to come up with Kristian> some solution, but please see below patch and see if you can suggest something Kristian> better. Kristian> Two issues: Kristian> 1. You added realloc_with_extra() / realloc_with_extra_if_needed(), and Kristian> used them to replace calls to realloc(). But realloc() as part of its Kristian> operation puts a terminating '\0' at the end of the allocation; with your Kristian> change this is now at a different (and not very useful) position in the Kristian> buffer. My change tries to put it in the original place, but it is not Kristian> terribly elegant, and perhaps your original change needs to be rethought. The \0 should not have been required at the end; In some cases it's good that it's there as we can avoid an extra realloc. Code that assumes that there is always an extra \0 at end is wrong code and need to be fixed; There is many different usage of the library that can cause the code not to end with \0 even before. When adding the above functions I did find some cases in MySQL where I had to change to use ->c_ptr_safe() instead of just ->ptr() but all those cases was a potential bug in the original code. Conclusion: Don't fix realloc. Instead fix the wrong call that is using the code! Kristian> 2. I found a terrible mess (IMHO): We have two distict, subtly differing, Kristian> sql_string.{h,cc}, one in /sql/ and one in /client/. We even seem to have a Kristian> requirement for ABI compatibility between the two, since mysql_embedded uses Kristian> client/sql_string.h with sql/sql_string.cc :-( And in fact your change seems Kristian> to break such ABI compatibility, so I suspect mysql_embedded might be subtly Kristian> broken in 5.2. Sergei did fix the above by making the sql_string.cc files identical. Kristian> But why do we need to have two sql_string in the first place? Especially given Kristian> that they are apparently sufficiently close to be ABI compatible ... can't we Kristian> eliminate on of them to get at least a little sanity into the sql_string Kristian> story? The reason was originally that sql_string.cc did at some point call other server code that was not in the client. This seams to not be the case anymore so now we can use identical versions of sql_string.cc Regards, Monty _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
