Re: bk commit into 5.0 tree (kaa:1.2516) BUG#29446

[Mats Kindahl]

Hi Alexey!

Comments below.

Just my few cents,
Mats Kindahl

Alexey Kopytov wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of kaa. When kaa does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-10-03 14:43:23+04:00, kaa@polly.(none) +4 -0
>   Issue a warning if a user sets an option or a variable to a value that is greater
> than a defined maximum for the option/variable.
>   
>   This is for bug #29446 "Specifying a myisam_sort_buffer > 4GB on 64 bit machines
> not possible". Support for myisam_sort_buffer_size > 4 GB on 64-bit Windows will be
> looked at later in 5.2.
>
>   mysql-test/r/variables.result@stripped, 2007-10-03 14:43:21+04:00, kaa@polly.(none) +2
> -0
>     Fixed the test.
>
>   mysql-test/t/variables.test@stripped, 2007-10-03 14:43:21+04:00, kaa@polly.(none) +1
> -0
>     Fixed the test.
>
>   mysys/my_getopt.c@stripped, 2007-10-03 14:43:21+04:00, kaa@polly.(none) +7 -0
>     Print a warning to the error log if a user sets an option to a value greater than
> the option's maximum value.
>
>   sql/set_var.cc@stripped, 2007-10-03 14:43:21+04:00, kaa@polly.(none) +15 -0
>     Issue an SQL warning if a user assignes a value greater than the variable's
> maximum value.
>
> diff -Nrup a/mysql-test/r/variables.result b/mysql-test/r/variables.result
> --- a/mysql-test/r/variables.result	2007-04-09 16:58:53 +04:00
> +++ b/mysql-test/r/variables.result	2007-10-03 14:43:21 +04:00
> @@ -218,6 +218,8 @@ show variables like 'net_buffer_length';
>  Variable_name	Value
>  net_buffer_length	1024
>  set net_buffer_length=2000000000;
> +Warnings:
> +Warning	1292	Truncated incorrect net_buffer_length value: '2000000000'
>  show variables like 'net_buffer_length';
>  Variable_name	Value
>  net_buffer_length	1048576
> diff -Nrup a/mysql-test/t/variables.test b/mysql-test/t/variables.test
> --- a/mysql-test/t/variables.test	2007-04-09 16:58:53 +04:00
> +++ b/mysql-test/t/variables.test	2007-10-03 14:43:21 +04:00
> @@ -139,6 +139,7 @@ show global variables like 'net_%';
>  show session variables like 'net_%';
>  set net_buffer_length=1;
>  show variables like 'net_buffer_length';
> +--warning 1292
>  set net_buffer_length=2000000000;
>  show variables like 'net_buffer_length';
>  
> diff -Nrup a/mysys/my_getopt.c b/mysys/my_getopt.c
> --- a/mysys/my_getopt.c	2007-02-22 17:59:54 +03:00
> +++ b/mysys/my_getopt.c	2007-10-03 14:43:21 +04:00
> @@ -730,7 +730,14 @@ static longlong getopt_ll(char *arg, con
>    num= eval_num_suffix(arg, err, (char*) optp->name);
>    if (num > 0 && (ulonglong) num > (ulonglong) optp->max_value
> &&
>   

Isn't it possible that this can wrap around in the case that the number 
is larger than 2 ** 64?

In that case, the error code from strtoll() should probably be checked, 
and an error message given when the value obviously cannot be 
represented internally.

Also, if a warning is given (i.e., processing continues), make sure that 
the value does not wrap around but instead saturates to the maximum 
value. (This is the case in the code below, but please add a test for 
this as well.)

>        optp->max_value) /* if max value is not set -> no upper limit */
> +  {
> +    char buf[22];
> +    my_getopt_error_reporter(WARNING_LEVEL,
> +                             "Truncated incorrect %s value: '%s'", 
> +                             optp->name, llstr(num, buf));
> +    
>      num= (ulonglong) optp->max_value;
> +  }
>    num= ((num - optp->sub_size) / block_size);
>    num= (longlong) (num * block_size);
>    return max(num, optp->min_value);
> diff -Nrup a/sql/set_var.cc b/sql/set_var.cc
> --- a/sql/set_var.cc	2007-07-30 19:27:30 +04:00
> +++ b/sql/set_var.cc	2007-10-03 14:43:21 +04:00
> @@ -1532,16 +1532,31 @@ bool sys_var_thd_ulong::check(THD *thd, 
>  bool sys_var_thd_ulong::update(THD *thd, set_var *var)
>  {
>    ulonglong tmp= var->save_result.ulonglong_value;
> +  char buf[22];
> +  bool truncated= false;
>  
>    /* Don't use bigger value than given with --maximum-variable-name=.. */
>    if ((ulong) tmp > max_system_variables.*offset)
> +  {
> +    truncated= true;
> +    llstr(tmp, buf);
>      tmp= max_system_variables.*offset;
> +  }
>  
>  #if SIZEOF_LONG == 4
>    /* Avoid overflows on 32 bit systems */
>    if (tmp > (ulonglong) ~(ulong) 0)
> +  {
> +    truncated= true;
> +    llstr(tmp, buf);
>      tmp= ((ulonglong) ~(ulong) 0);
> +  }
>  #endif
> +  if (truncated)
> +    push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
> +                        ER_TRUNCATED_WRONG_VALUE,
> +                        ER(ER_TRUNCATED_WRONG_VALUE), name,
> +                        buf);
>  
>    if (option_limits)
>      tmp= (ulong) getopt_ull_limit_value(tmp, option_limits);
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com