List:Commits« Previous MessageNext Message »
From:Dmitry Shulga Date:January 19 2011 6:37am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026
View as plain text  
Hi Davi!

On 18.01.2011, at 15:42, Davi Arnaut wrote:

>>> Hum, since the hook is global, there isn't much reason to pass the
>>> stack check function each time we need to compile a expression.
>>> 
>>> Just add the hook to regcomp.c with internal linkage:
>>> 
>>> static my_regex_stack_check_t stack_check;
>>> 
>>> and we let a hook be registered at my_regex_init time:
>>> 
>>> my_regex_init(CHARSET_INFO *cs, my_regex_stack_check_t
>>> stack_check);
>>> 
>>> In
I still don't fully understand your point.
Do you suggest moved definition of function check_enough_stack_size() from
sql/item_cmpfunc.cc to regex/regcomp.c and make this function as static?
But this solution add extra dependency between regex and sql. I believe that this
dependency is superfluous.
Moreover,  check_enough_stack_size() uses current_thd that is not available inside regex. 
 

>>> this case, my_regex_init sets the hook to the given one.
>> I don't agree with your suggestion. First, because solution with
>> explicit argument is more meaningful. With your approach usage of
>> check_stack_overrun function is implicit and hidden inside body of
>> my_regex_init/my_regcomp. In my approach check_stack_overrun passed
>> explicitly and developer reading this source code clearly understand
>> what I do there.
> 
> If we followed this line of thought, the same argument could be used for any
> initialization that is done through my_regex_init.
> 
> Besides, what I suggest is a already established pattern in MySQL code.
Established pattern is a good argument. But I think that for this case this approach is
not suitable.

> For example, look at how the error hooks are routed back into the server, should we
> add a argument to every my_error just because it might be hidden inside the body of
> my_error? Sorry, but your argument is not sound. Functions do implicit things, that's what
> they are useful for.


> 
>> Second, because current_thd is not defined for some client programs,
>> mysqltest for example.
> 
> The hook won't be set in these programs.
All of these programs calls my_regcomp(). I don't understand how you are going to
differentiate inside my_regcomp() whether to install a hook or not. 


Regards, Dmitry.

Thread
bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026Dmitry Shulga24 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)Bug#58026Davi Arnaut17 Jan
    • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026Dmitry Shulga18 Jan
      • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)Bug#58026Davi Arnaut18 Jan
        • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026Dmitry Shulga19 Jan
          • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)Bug#58026Davi Arnaut19 Jan