Re: bzr commit into mysql-5.1 branch (ingo.struewing:2712) Bug#39277

[Mattias Jonsson]

Hi,

Good commit message, informative code comment!

Patch verified on Mac OSX 10.5.

Patch OK to push!

/Mattias

Ingo Struewing wrote:
> #At file:///home2/mydev/bzrroot/mysql-5.1-bug39277/
> 
>  2712 Ingo Struewing	2008-11-14
>       Bug#39277 - symlink.test fails on Debian
>       
>       When the data directory contained a symbolic link to another
>       file system, and the DATA or INDEX DIRECTORY clause of a
>       CREATE TABLE statement referred to a subdirectory of the data
>       directory, this was accepted.
>       
>       The problem was the use of a table file path name, which included
>       the table name without an extension, for the comparison against
>       the data directory path name. This was almost always a
>       non-existent file. The internal algorithm failed to resolve
>       symbolic links for non-existent files. So we compared unrelated
>       path names.
>       
>       Fixed by truncating the table name from the path before resolving
>       symlinks.
> modified:
>   sql/sql_table.cc
> 
> per-file messages:
>   sql/sql_table.cc
>     Bug#39277 - symlink.test fails on Debian
>     Changed test for data directory to exclude the table name from the
>     comparison.
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc	2008-11-13 13:09:22 +0000
> +++ b/sql/sql_table.cc	2008-11-14 12:05:19 +0000
> @@ -3518,15 +3518,43 @@ bool mysql_create_table_no_lock(THD *thd
>    create_info->table_existed= 0;		// Mark that table is created
>  
>  #ifdef HAVE_READLINK
> -  if (test_if_data_home_dir(create_info->data_file_name))
>    {
> -    my_error(ER_WRONG_ARGUMENTS, MYF(0), "DATA DIRECTORY");
> -    goto unlock_and_end;
> -  }
> -  if (test_if_data_home_dir(create_info->index_file_name))
> -  {
> -    my_error(ER_WRONG_ARGUMENTS, MYF(0), "INDEX DIRECTORY");
> -    goto unlock_and_end;
> +    size_t dirlen;
> +    char   dirpath[FN_REFLEN];
> +
> +    /*
> +      data_file_name and index_file_name include the table name without
> +      extension. Mostly this does not refer to an existing file. When
> +      comparing data_file_name or index_file_name against the data
> +      directory, we try to resolve all symbolic links. On some systems,
> +      we use realpath(3) for the resolution. This returns ENOENT if the
> +      resolved path does not refer to an existing file. my_realpath()
> +      does then copy the requested path verbatim, without symlink
> +      resolution. Thereafter the comparison can fail even if the
> +      requested path is within the data directory. E.g. if symlinks to
> +      another file system are used. To make realpath(3) return the
> +      resolved path, we strip the table name and compare the directory
> +      path only. If the directory doesn't exist either, table creation
> +      will fail anyway.
> +    */
> +    if (create_info->data_file_name)
> +    {
> +      dirname_part(dirpath, create_info->data_file_name, &dirlen);
> +      if (test_if_data_home_dir(dirpath))
> +      {
> +        my_error(ER_WRONG_ARGUMENTS, MYF(0), "DATA DIRECTORY");
> +        goto unlock_and_end;
> +      }
> +    }
> +    if (create_info->index_file_name)
> +    {
> +      dirname_part(dirpath, create_info->index_file_name, &dirlen);
> +      if (test_if_data_home_dir(dirpath))
> +      {
> +        my_error(ER_WRONG_ARGUMENTS, MYF(0), "INDEX DIRECTORY");
> +        goto unlock_and_end;
> +      }
> +    }
>    }
>  
>  #ifdef WITH_PARTITION_STORAGE_ENGINE
> 
>