Description:
Valgrind detects two problems in DBUG_PRINT code when running replication code with debug and valgrind enabled:
1. In Relay_log_info::wait_for_gtid_set, in the following lines:
DBUG_PRINT("info", ("Waiting for '%s'. is_subset: %d and "
"!is_intersection_nonempty: %d",
wait_gtid_set->to_string(), wait_gtid_set->is_subset(executed_gtids),
!owned_gtids->is_intersection_nonempty(wait_gtid_set)));
wait_gtid_set->to_string will return newly allocated memory which is never freed.
2. In MYSQL_BIN_LOG::init_gtid_sets, in the following lines:
reached_first_file= (rit == filename_list.rend());
DBUG_PRINT("info", ("filename='%s' reached_first_file=%d",
rit->c_str(), reached_first_file));
In case rit==filename_list.end(), rit->c_str() is undefined so it may print garbage.
How to repeat:
Compile with both Valgrind and debug mode, or read the code.
Suggested fix:
The first problem may be hard to spot by reading the code. It just prints the return value from the Gtid_set::to_string() function, which may not look suspicious enough. OTOH, there is already a function Gtid_set::to_string(char **,...) which does the same thing as Gtid_set::to_string() but returns the value through its first parameter. This seems less error-prone, since the caller has to store the value in a local variable, and then it is more obvious that it needs to be freed too. So probably good to remove Gtid_set::to_string() and keep Gtid_set::to_string(char**,...) to reduce the risk for similar bugs in the future.
This bug was found when debugging read_gtids_from_binlog, when adding debug printouts for Gtid_sets there. These printouts would be useful to keep, so including them in this patch too.
Description: Valgrind detects two problems in DBUG_PRINT code when running replication code with debug and valgrind enabled: 1. In Relay_log_info::wait_for_gtid_set, in the following lines: DBUG_PRINT("info", ("Waiting for '%s'. is_subset: %d and " "!is_intersection_nonempty: %d", wait_gtid_set->to_string(), wait_gtid_set->is_subset(executed_gtids), !owned_gtids->is_intersection_nonempty(wait_gtid_set))); wait_gtid_set->to_string will return newly allocated memory which is never freed. 2. In MYSQL_BIN_LOG::init_gtid_sets, in the following lines: reached_first_file= (rit == filename_list.rend()); DBUG_PRINT("info", ("filename='%s' reached_first_file=%d", rit->c_str(), reached_first_file)); In case rit==filename_list.end(), rit->c_str() is undefined so it may print garbage. How to repeat: Compile with both Valgrind and debug mode, or read the code. Suggested fix: The first problem may be hard to spot by reading the code. It just prints the return value from the Gtid_set::to_string() function, which may not look suspicious enough. OTOH, there is already a function Gtid_set::to_string(char **,...) which does the same thing as Gtid_set::to_string() but returns the value through its first parameter. This seems less error-prone, since the caller has to store the value in a local variable, and then it is more obvious that it needs to be freed too. So probably good to remove Gtid_set::to_string() and keep Gtid_set::to_string(char**,...) to reduce the risk for similar bugs in the future. This bug was found when debugging read_gtids_from_binlog, when adding debug printouts for Gtid_sets there. These printouts would be useful to keep, so including them in this patch too.