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.
