Hi Alfranio,
Check some comments in-line.
On Thu, 2009-04-09 at 21:52 +0000, Alfranio Correia wrote:
> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-43949/mysql-5.1-bugteam/
> based on revid:v.narayanan@stripped
>
> 2862 Alfranio Correia 2009-04-09
> BUG#43949 Initialization of slave produces a warning message in Valgrind
>
> In order to define the --slave-load-tmpdir, the init_relay_log_file()
> was calling fn_format(MY_PACK_FILENAME) which internally was calling
SUGGESTION:
was indirectly calling strmov_overlapp (through pack_dirname)
> strmov_overlapp() and the following warning message was being printed
> out while running in Valgrind: "source and destination overlap in strcpy".
>
> We fixed the issue by removing the flag MY_PACK_FILENAME as it was not
> necessary. In a nutshell, with this flag the function fn_format() tried
> to replace a directory by either "~", "." or "..". However, we wanted
> exactly the opposite which is achieved by the flag MY_UNPACK_FILENAME.
>
> In this patch, we also refactored the functions init_relay_log_file()
> and check_temp_dir(). The former was refactored to call the fn_format()
> with the flag MY_SAFE_PATH along with two more flags, MY_UNPACK_FILENAME
SUGGESTION:
with an extra flag: MY_SAFE_PATH...
> and MY_RETURN_REAL_PATH, in order to avoid issues with long directories,
> get rid off "~", "." and ".." and return an absolute path, respectively.
> This result is stored in an rli variable, which is then processed by the
> other function in order to verify if the directory exists and if we are
> able to create files in it.
> modified:
> sql/rpl_rli.cc
> sql/slave.cc
>
> === modified file 'sql/rpl_rli.cc'
> --- a/sql/rpl_rli.cc 2009-02-22 13:40:52 +0000
> +++ b/sql/rpl_rli.cc 2009-04-09 21:52:44 +0000
> @@ -105,7 +105,7 @@ int init_relay_log_info(Relay_log_info*
> rli->tables_to_lock_count= 0;
>
> fn_format(rli->slave_patternload_file, PREFIX_SQL_LOAD, slave_load_tmpdir, "",
> - MY_PACK_FILENAME | MY_UNPACK_FILENAME |
> + MY_SAFE_PATH | MY_UNPACK_FILENAME |
> MY_RETURN_REAL_PATH);
REQUIREMENT:
fn_format may return NULL with MY_SAFE_PATH. Please handle this.
> rli->slave_patternload_file_size= strlen(rli->slave_patternload_file);
>
>
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc 2009-04-08 10:07:24 +0000
> +++ b/sql/slave.cc 2009-04-09 21:52:44 +0000
> @@ -2638,14 +2638,21 @@ err:
> LOAD DATA INFILE.
> */
> static
> -int check_temp_dir(char* tmp_dir, char *tmp_file)
> +int check_temp_dir(char* tmp_file)
> {
> int fd;
> MY_DIR *dirp;
> + char tmp_dir[FN_REFLEN];
> + size_t tmp_dir_size;
>
> DBUG_ENTER("check_temp_dir");
>
> /*
> + Get the directory from the temporary file.
> + */
> + dirname_part(tmp_dir, tmp_file, &tmp_dir_size);
> +
> + /*
> Check if the directory exists.
> */
> if (!(dirp=my_dir(tmp_dir,MYF(MY_WME))))
> @@ -2800,7 +2807,7 @@ log '%s' at position %s, relay log '%s'
>
> llstr(rli->group_master_log_pos,llbuff),rli->group_relay_log_name,
> llstr(rli->group_relay_log_pos,llbuff1));
>
> - if (check_temp_dir(slave_load_tmpdir, rli->slave_patternload_file))
> + if (check_temp_dir(rli->slave_patternload_file))
> {
> rli->report(ERROR_LEVEL, thd->main_da.sql_errno(),
> "Unable to use slave's temporary directory %s - %s",
>
>
Good work,
Luís
--
Luís
