MDEV-40996 Support --sync_with_master 0, $variable in mysqltest - #5619
MDEV-40996 Support --sync_with_master 0, $variable in mysqltest#5619ParadoxV5 wants to merge 1 commit into
--sync_with_master 0, $variable in mysqltest#5619Conversation
|
@ParadoxV5 mtr/mariadb-test changes can (and should) go into the earliest version, 10.11. They don't affect the server or production use, and it's rather annoying to remember what mtr feature was added in what version and adjust tests when merging up. Better keep mtr/mariadb-test the same everywhere whenever possible |
| buff= (char*)my_malloc(PSI_NOT_INSTRUMENTED, strlen(p)+1, | ||
| MYF(MY_WME|MY_FAE)); | ||
| get_string(&buff, &p, command); | ||
| start= get_string(&buff, &p, command); |
There was a problem hiding this comment.
start does double duty here, it is both the value passed to do_sync_with_master2() and the pointer freed at 5405. when the argument is a $variable, get_string() returns var->str_val, so my_free(start) frees the variable's own string. also the malloc buffer leaks too, since get_string() moved buff to to+1.
--sync_with_master 0,$conn_name aborts under ASAN at exit
==117736==ERROR: AddressSanitizer: heap-use-after-free
#0 my_free mysys/my_malloc.c:204
#1 var_free client/mysqltest.cc:2796
#4 free_used_memory() client/mysqltest.cc:1878
#5 cleanup_and_exit client/mysqltest.cc:1926
so you can keep start assigned the allocation base alongside buff, to free at the end, and add a separate variable that takes the get_string() return and is passed to do_sync_with_master2(). The free then always targets this function's own buffer.
There was a problem hiding this comment.
🫠 thank you
This is so hard to unsee now that I see it:
if (buff)
my_free(start);There was a problem hiding this comment.
Done, @KhaledR57.
I’ve had fun here and will be on vacation starting the day after tomorrow, so feel free to take the branch over, GSoC colleague 😀.
`--sync_with_master` uses `get_string()`, which has `$variable` support, but it only uses the read buffer, which is written with the unexpanded string and not the variable value. Reviewed-by: KhaledR57 <khaled57.dev@gmail.com>
153ee96 to
8b8d062
Compare
ParadoxV5
changed the title
--sync_with_master 0, $variable--sync_with_master 0, $variable in mysqltest
3 participants
--sync_with_masterusesget_string(), which has$variablesupport, but it only uses the read buffer, which is written with the unexpanded string and not the variable value.