Hi,
I have a few comments on this patch:
kernel.cc @@141
---------------
Should the comment read "needed since we cannot trust that the path has
*not* become invalid"? Or remove the double negation :)
stream.cc
---------
BACKUP ... TO '../backup1.bak'; seems to ignore the "../". I *think*
this is because in Stream::make_relative_path, j is set to
backupdir->length(), and it therefore points outside the string, not to
'/'. "if (base[j] == FN_LIBCHAR)" is therefore not true the first
iteration. Because of this, the first ../ in orig_loc does not have any
effect.
mysqld.cc
---------
Are the warnings supposed to be localized?
set_var.cc
----------
Documentation for sys_check_backupdir - I don't think this function can
return anything but 0.
backup_backupdir.test:
----------------------
It would be somewhat safer to use another backup image name on the ../
tests, something like '../bup_backupdir2.bak'. This would more likely
result in an error if "../" handling was not correct.
Surprisingly, the test with path ../../../../../../ completes
successfully on my Linux system. The number of ../ is much higher than
the path depth. When I try to do the same manually, the command fails as
expected.
Chuck Bell wrote:
> #At file:///D:/source/bzr/mysql-6.0-bug-35230/
>
> 2674 Chuck Bell 2008-07-31
> BUG#35230 Backup: no backupdir
>
> This patch adds the dynamic variable --backupdir which specifies the path
> where the image files are placed (backup) or accessed (restore) if a path
> is not specified in the command.
> added:
> mysql-test/r/backup_backupdir.result
> mysql-test/t/backup_backupdir.test
> modified:
> mysql-test/r/backup_progress.result
> mysql-test/t/backup_progress.test
> sql/backup/backup_kernel.h
> sql/backup/kernel.cc
> sql/backup/stream.cc
> sql/backup/stream.h
> sql/mysqld.cc
> sql/set_var.cc
> sql/set_var.h
> sql/share/errmsg.txt
> sql/sql_parse.cc
--
Jørgen Løland
