Re: bk commit into 5.1 tree (istruewing:1.2576)

[Ingo Strüwing]

Hi Chuck,

thanks for the review. See comments below.

Chuck Bell, 02.04.2008 18:14:
> Ingo,
> 
> The patch causes compiler warnings on Windows:
> 
> 2>sql_plugin.cc
> 2>.\sql_plugin.cc(3211) : warning C4291: 'void *sys_var_pluginvar::operator
> new(size_t,MEM_ROOT *)' : no matching operator delete found; memory will not
> be freed if initialization throws an exception
> 2>        .\sql_plugin.cc(166) : see declaration of
> 'sys_var_pluginvar::operator new'
> 2>.\sql_plugin.cc(3223) : warning C4291: 'void *sys_var_pluginvar::operator
> new(size_t,MEM_ROOT *)' : no matching operator delete found; memory will not
> be freed if initialization throws an exception
> 2>        .\sql_plugin.cc(166) : see declaration of
> 'sys_var_pluginvar::operator new'

This is outside the scope of my patch. I don't have a quick fix for it.
Either the warnings are bogus or we need need to add optional freeing of
system variables when deleting the sys_var_chain.

> 
> As you said, the federated tests fail until windows. Please add the
> following to all federated tests (before the federated include). This will
> keep the tests from running on Windows until such time it can be fixed on
> Windows.
> 
> --source include/not_windows.inc

Ok.

> 
> Can we not use the String class? Will that make the dynstr_append_mem thing
> easier to deal with?

IMHO the String class it too fat for this purpose. It probably would do
unnecessary reallocations. Let the second reviewer vote between us.

> 
> Chuck
> 
>> -----Original Message-----
>> From: Ingo Struewing [mailto:ingo@stripped] 
>> Sent: Tuesday, April 01, 2008 15:43 PM
>> To: commits@stripped
>> Subject: bk commit into 5.1 tree (istruewing:1.2576)
>>
>> Below is the list of changes that have just been committed 
...

Can you please cut such long emails when replying?

Regards
Ingo
-- 
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Dachauer Str. 37, D-80335 München
Geschäftsführer: Kaj Arnö - HRB München 162140