2710 Chuck Bell 2008-10-14
BUG#39598 Valgrind warnings in backup_functions and backup_procedures
Patch refactors prepare_path() method in stream class to use
standard server methods instead of processing paths manually.
modified:
sql/backup/stream.cc
sql/backup/stream.h
=== modified file 'sql/backup/stream.cc'
--- a/sql/backup/stream.cc 2008-10-07 10:26:19 +0000
+++ b/sql/backup/stream.cc 2008-10-14 20:46:34 +0000
@@ -204,74 +204,6 @@ Stream::Stream(Logger &log, ::String *ba
}
/**
- Make a relative path.
-
- This method takes the backupdir and the path specified on the backup command
- (orig_loc) and forms a combined path. It walks the backupdir from the right
- and the orig_loc from the left to position the paths for concatenation. Output
- is written to new_path.
-
- @param[OUT] new_path The newly combined path + file name.
- @param[IN] orig_loc The path + file name specified in the backup command.
- @param[IN] backupdir The backupdir system variable value.
-
- For example, if backupdir = '/dev/tmp' and orig_log = '../backup.bak', the
- combined path is = '/dev/backup.bak'.
-
- Note: fn_format may likely be used instead of this function - will
- not be changed unless bugs are found.
-
- @returns
- 0 if success
- 1 if cannot be combined. Note: m_path is set to '' when this occurs to
- trigger error in call stack.
-*/
-int Stream::make_relative_path(char *new_path,
- char *orig_loc,
- ::String *backupdir)
-{
- char fixed_base[FN_LEN];
- char fixed_rel[FN_LEN];
- cleanup_dirname(fixed_base, backupdir->c_ptr());
- cleanup_dirname(fixed_rel, orig_loc);
- char *rel;
- char *base= fixed_base;
- bool done= FALSE;
- char *found= fixed_rel;
- int j= backupdir->length() - 1;
- new_path[0]= 0; // initialize the new path to an empty string
-
- /*
- For each '../' in orig_loc, move the pointer to the right for rel.
- For each '../' in orig_loc, move pointer to the left for base.
- */
- while (!done)
- {
- rel= found; // save last known position
- // find location of next level in relative path
- found= strstr(found, FN_PARENTDIR);
- if (found)
- {
- found+= 2; // move past '..\'
- if (base[j] == FN_LIBCHAR)
- j--; // move past last '\'
- if (j == 0) // We are trying to move too far down the path
- return 1;
- /*
- Look for the next level down.
- */
- while ((base[j] != FN_LIBCHAR) && j)
- j--;
- }
- else
- done= TRUE;
- }
- strcpy (new_path, base);
- strcpy (new_path + j, rel);
- return 0;
-}
-
-/**
Prepare path for access.
This method takes the backupdir and the file name specified on the backup
@@ -289,101 +221,59 @@ int Stream::make_relative_path(char *new
int Stream::prepare_path(::String *backupdir,
LEX_STRING orig_loc)
{
- int path_len= 0;
+ char fix_path[FN_REFLEN];
+ char full_path[FN_REFLEN];
/*
- The full path is needed to test for secure-file-priv option in
- Stream::open. If not full, replace backupdir with the full
- path name, which is relative to datadir (i.e., global variable
- mysql_real_data_home)
- */
- if (!test_if_hard_path(backupdir->c_ptr()))
- {
- char full_path[FN_LEN];
-
- /*
- MY_UNPACK_FILENAME "~/" will be changed to full path
- MY_RELATIVE_PATH path is constructed from the base path
- mysql_real_data_home combined with the
- relative path backupdir. Also handles
- "../" in backupdir and converts dirname
- to fit this system
- */
- fn_format(full_path, backupdir->c_ptr(), mysql_real_data_home, "",
- MY_UNPACK_FILENAME | MY_RELATIVE_PATH);
-
- uint32 full_path_len= strlen(full_path);
- backupdir->copy(full_path, full_path_len, &::my_charset_bin);
- }
-
- /*
Prepare the path using the backupdir iff no relative path
or no hard path included.
Relative paths are formed from the backupdir system variable.
+
+ Case 1: Backup image file name has relative path.
+ Make relative to backupdir.
+
+ Example BACKUP DATATBASE ... TO '../monthly/dec.bak'
+ If backupdir = '/dev/daily' then the
+ calculated path becomes
+ '/dev/monthly/dec.bak'
+
+ Case 2: Backup image file name has no path or has a subpath.
+
+ Example BACKUP DATABASE ... TO 'week2.bak'
+ If backupdir = '/dev/weekly/' then the
+ calculated path becomes
+ '/dev/weekly/week2.bak'
+ Example BACKUP DATABASE ... TO 'jan/day1.bak'
+ If backupdir = '/dev/monthly/' then the
+ calculated path becomes
+ '/dev/monthly/jan/day1.bak'
+
+ Case 3: Backup image file name has hard path.
+
+ Example BACKUP DATATBASE ... TO '/dev/dec.bak'
+ If backupdir = '/dev/daily/backup' then the
+ calculated path becomes
+ '/dev/dec.bak'
+ */
+
+ /*
+ First, we construct the complete path from backupdir.
+ */
+ fn_format(fix_path, backupdir->ptr(), mysql_real_data_home, "",
+ MY_UNPACK_FILENAME | MY_RELATIVE_PATH);
+
+ /*
+ Next, we contruct the full path to the backup file.
+ */
+ fn_format(full_path, orig_loc.str, fix_path, "",
+ MY_UNPACK_FILENAME | MY_RELATIVE_PATH);
+
+ /*
+ Copy result to member variable for Stream class.
*/
- if (is_prefix(orig_loc.str, FN_PARENTDIR))
- {
- /*
- Case 1: Backup image file name has relative path.
- Make relative to backupdir.
-
- Example BACKUP DATATBASE ... TO '../monthly/dec.bak'
- If backupdir = '/dev/daily' then the
- calculated path becomes
- '/dev/monthly/dec.bak'
- */
- char new_path[FN_LEN];
- if (make_relative_path(new_path, orig_loc.str, backupdir))
- m_path.length(0);
- path_len= strlen(new_path) + 1;
- m_path.alloc(path_len);
- m_path.length(0);
- m_path.append(new_path);
- }
- else if (!test_if_hard_path(orig_loc.str))
- {
- /*
- Case 2: Backup image file name has no path or has a subpath.
-
- Example BACKUP DATABASE ... TO 'week2.bak'
- If backupdir = '/dev/weekly/' then the
- calculated path becomes
- '/dev/weekly/week2.bak'
- Example BACKUP DATABASE ... TO 'jan/day1.bak'
- If backupdir = '/dev/monthly/' then the
- calculated path becomes
- '/dev/monthly/jan/day1.bak'
- */
- path_len=backupdir->length() + orig_loc.length + 1;
- m_path.alloc(path_len);
- fn_format(m_path.c_ptr(), orig_loc.str, backupdir->c_ptr(), "",
- MY_UNPACK_FILENAME | MY_RELATIVE_PATH);
- }
- else
- {
- /*
- Case 3: Backup image file name has hard path.
-
- Example BACKUP DATATBASE ... TO '/dev/dec.bak'
- If backupdir = '/dev/daily/backup' then the
- calculated path becomes
- '/dev/dec.bak'
- */
- path_len= orig_loc.length + 1;
- int dn_length= dirname_length(orig_loc.str);
-
- m_path.alloc(path_len);
- m_path.length(0);
- m_path.append(orig_loc.str, dn_length); // Append directory-part only
-
- // Convert directory name to fit this system
- convert_dirname(m_path.c_ptr(), m_path.c_ptr(), m_path.c_ptr() + dn_length);
-
- // Append filename now that directory name has been converted
- m_path.append(orig_loc.str + dn_length);
- }
- m_path.length(path_len);
+ m_path.copy(full_path, strlen(full_path), system_charset_info);
+
return 0;
}
=== modified file 'sql/backup/stream.h'
--- a/sql/backup/stream.h 2008-10-07 10:26:19 +0000
+++ b/sql/backup/stream.h 2008-10-14 20:46:34 +0000
@@ -104,9 +104,6 @@ class Stream: public fd_stream
private:
- int make_relative_path(char *new_path,
- char *orig_loc,
- ::String *backupdir);
int prepare_path(::String *backupdir,
LEX_STRING orig_loc);
bool test_secure_file_priv_access(char *path);
| Thread |
|---|
| • bzr push into mysql-6.0-rpl branch (cbell:2710) Bug#39598 | Chuck Bell | 15 Oct |
