Patch looks good. I approve it. A minor suggestion below.
--
Øystein
Chuck Bell wrote:
> #At file:///D:/source/bzr/mysql-6.0-bug-39127/
>
> 2693 Chuck Bell 2008-09-11
> BUG#39127 Backup fails on PowerMac platform
>
> The PowerMacG5 platform does not reference the variables used in the
> int8store() macro as
> originally written. The address of write_id is invalid forcing the system to
> crash.
>
> This patch fixes the problem by using pointers to allocated memory for the
> int8store()
> and uint8korr() as type uchar * instead of ulonglong.
> modified:
> sql/log.cc
>
> per-file messages:
> sql/log.cc
> Fixed problem on PowerMac platform with local variables and int8store() macro.
> === modified file 'sql/log.cc'
> --- a/sql/log.cc 2008-08-28 15:13:31 +0000
> +++ b/sql/log.cc 2008-09-11 18:21:54 +0000
> @@ -3515,6 +3515,8 @@ my_bool MYSQL_BACKUP_LOG::check_backup_l
> ulonglong MYSQL_BACKUP_LOG::get_next_backup_id()
> {
> ulonglong id= 0;
> + uchar *read_id= 0;
> + uchar *write_id= 0;
> char buff[FN_REFLEN], *file_path;
> File file= 0;
>
> @@ -3538,7 +3540,6 @@ ulonglong MYSQL_BACKUP_LOG::get_next_bac
> file= my_open(file_path, O_RDWR|O_BINARY|O_CREAT, MYF(MY_WME));
> if (!file)
> goto err_end;
> -
> if (my_fstat(file, &state, MYF(0)))
> goto err;
> /*
> @@ -3562,10 +3563,10 @@ ulonglong MYSQL_BACKUP_LOG::get_next_bac
> // else .... we read the next value in the file!
> else
> {
> - ulonglong read_id= 0;
> my_seek(file, 0, 0, MYF(MY_WME));
> - my_read(file, (uchar *)&read_id, sizeof(ulonglong), MYF(MY_WME|MY_NABP));
> - id= uint8korr(&read_id);
> + read_id= (uchar *)my_malloc(sizeof(ulonglong), MYF(MY_ZEROFILL));
> + my_read(file, read_id, sizeof(ulonglong), MYF(MY_WME|MY_NABP));
> + id= uint8korr(read_id);
> id++;
> }
> }
> @@ -3579,8 +3580,6 @@ ulonglong MYSQL_BACKUP_LOG::get_next_bac
> */
> if ((m_next_id != id) && id)
> {
> - ulonglong write_id= 0;
> -
> if (!file)
> {
> file_path= make_backup_log_name(buff, BACKUP_SETTINGS_NAME.str, ".obx");
> @@ -3589,8 +3588,9 @@ ulonglong MYSQL_BACKUP_LOG::get_next_bac
> goto err_end;
> }
> my_seek(file, 0, 0, MYF(MY_WME));
> - int8store(&write_id, id);
> - my_write(file, (uchar *)&write_id, sizeof(ulonglong), MYF(MY_WME));
> + write_id= (uchar *)my_malloc(sizeof(ulonglong), MYF(MY_ZEROFILL));
> + int8store(write_id, id);
> + my_write(file, write_id, sizeof(ulonglong), MYF(MY_WME));
> }
> err:
> if (file > 0)
> @@ -3600,6 +3600,10 @@ err_end:
> m_next_id= id;
> pthread_mutex_unlock(&LOCK_backupid);
> DBUG_PRINT("backup_log",("The next id is %lu.\n", (ulong)id));
> + if(read_id)
> + my_free(read_id, MYF(0));
> + if(write_id)
> + my_free(write_id, MYF(0));
> return id;
> }
>
>
>
Why put the free calls in err_end? Why not call my_free right after it
has been used? There is no jumps to err_end between allocation and usage.
--
Øystein
