From: Dmitry Shulga
Date: January 19 2011 6:37am
Subject: Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026
List-Archive: http://lists.mysql.com/commits/129135
Message-Id: <A35A1024-0560-48DA-855B-6033C2D666AD@oracle.com>
MIME-Version: 1.0 (Apple Message framework v1082)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable

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.
>>>=20
>>> Just add the hook to regcomp.c with internal linkage:
>>>=20
>>> static my_regex_stack_check_t stack_check;
>>>=20
>>> and we let a hook be registered at my_regex_init time:
>>>=20
>>> my_regex_init(CHARSET_INFO *cs, my_regex_stack_check_t
>>> stack_check);
>>>=20
>>> 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.=20
=20

>>> 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.
>=20
> If we followed this line of thought, the same argument could be used =
for any initialization that is done through my_regex_init.
>=20
> 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.


>=20
>> Second, because current_thd is not defined for some client programs,
>> mysqltest for example.
>=20
> 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.=20


Regards, Dmitry.