Hi Monty, While fixing test failures in the MariaDB 5.5 tree, I encountered problems in sql_string, related to changes you did there in 5.2. I managed to come up with some solution, but please see below patch and see if you can suggest something better. Two issues: 1. You added realloc_with_extra() / realloc_with_extra_if_needed(), and used them to replace calls to realloc(). But realloc() as part of its operation puts a terminating '\0' at the end of the allocation; with your change this is now at a different (and not very useful) position in the buffer. My change tries to put it in the original place, but it is not terribly elegant, and perhaps your original change needs to be rethought. 2. I found a terrible mess (IMHO): We have two distict, subtly differing, sql_string.{h,cc}, one in /sql/ and one in /client/. We even seem to have a requirement for ABI compatibility between the two, since mysql_embedded uses client/sql_string.h with sql/sql_string.cc :-( And in fact your change seems to break such ABI compatibility, so I suspect mysql_embedded might be subtly broken in 5.2. But why do we need to have two sql_string in the first place? Especially given that they are apparently sufficiently close to be ABI compatible ... can't we eliminate on of them to get at least a little sanity into the sql_string story? - Kristian. knielsen@knielsen-hq.org writes:
At http://bazaar.launchpad.net/~maria-captains/maria/5.5
------------------------------------------------------------ revno: 3170 revision-id: knielsen@knielsen-hq.org-20111209144027-yf5as4v9lb6jt3s7 parent: knielsen@knielsen-hq.org-20111208170848-pijq4i8ceqwcl19q committer: knielsen@knielsen-hq.org branch nick: mariadb-5.5 timestamp: Fri 2011-12-09 15:40:27 +0100 message: After-merge fixes for 5.5 merge.
Fix typo causing too low timeout value for wait_for_slave_param.inc. Fix wrong/missing '\0' termination in String:realloc() after Monty's optimisations to reduce reallocation calls. === modified file 'mysql-test/include/wait_for_slave_param.inc' --- a/mysql-test/include/wait_for_slave_param.inc 2010-12-20 14:15:01 +0000 +++ b/mysql-test/include/wait_for_slave_param.inc 2011-12-09 14:40:27 +0000 @@ -79,7 +79,7 @@ if ($_slave_check_configured == 'No such
# mysqltest doesn't provide any better way to multiply by 10 --let $_wait_for_slave_param_zero= 0 ---let $_slave_timeout_counter= $_slave_timeout$zero +--let $_slave_timeout_counter= $_slave_timeout$_wait_for_slave_param_zero --let $_slave_continue= 1 while ($_slave_continue) {
=== modified file 'sql/rpl_handler.cc' --- a/sql/rpl_handler.cc 2011-11-03 18:17:05 +0000 +++ b/sql/rpl_handler.cc 2011-12-09 14:40:27 +0000 @@ -397,7 +397,7 @@ int Binlog_transmit_delegate::before_sen
int ret= 0; FOREACH_OBSERVER(ret, before_send_event, thd, - (¶m, (uchar *)packet->c_ptr(), + (¶m, (uchar *)packet->c_ptr_safe(), packet->length(), log_file+dirname_length(log_file), log_pos)); return ret; @@ -411,7 +411,7 @@ int Binlog_transmit_delegate::after_send
int ret= 0; FOREACH_OBSERVER(ret, after_send_event, thd, - (¶m, packet->c_ptr(), packet->length())); + (¶m, packet->c_ptr_safe(), packet->length())); return ret; }
=== modified file 'sql/sql_string.cc' --- a/sql/sql_string.cc 2011-11-03 18:17:05 +0000 +++ b/sql/sql_string.cc 2011-12-09 14:40:27 +0000 @@ -80,8 +80,12 @@ bool String::real_alloc(uint32 length) no allocation occured.
@retval true An error occured when attempting to allocate memory. + + The _internal() version is just to share code between realloc() and + realloc_with_extra(), so that we can put the NULL termination at the right + position. */ -bool String::realloc(uint32 alloc_length) +bool String::realloc_internal(uint32 alloc_length) { if (Alloced_length <= alloc_length) { @@ -109,6 +113,13 @@ bool String::realloc(uint32 alloc_length Ptr= new_ptr; Alloced_length= len; } + return FALSE; +} + +bool String::realloc(uint32 alloc_length) +{ + if (realloc_internal(alloc_length)) + return TRUE; Ptr[alloc_length]=0; // This make other funcs shorter return FALSE; }
=== modified file 'sql/sql_string.h' --- a/sql/sql_string.h 2011-11-03 18:17:05 +0000 +++ b/sql/sql_string.h 2011-12-09 14:40:27 +0000 @@ -255,12 +255,18 @@ public: return real_alloc(arg_length); } bool real_alloc(uint32 arg_length); // Empties old string +private: + bool realloc_internal(uint32 arg_length); +public: bool realloc(uint32 arg_length); bool realloc_with_extra(uint32 arg_length) { if (extra_alloc < 4096) extra_alloc= extra_alloc*2+128; - return realloc(arg_length + extra_alloc); + if (realloc_internal(arg_length + extra_alloc)) + return TRUE; + Ptr[arg_length]= 0; + return FALSE; } bool realloc_with_extra_if_needed(uint32 arg_length) {
_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits