maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Sergei,


On 01/27/2014 05:52 PM, Sergei Golubchik wrote:
> Hi, Alexander!
>
> On Jan 27, Alexander Barkov wrote:
> > On 01/26/2014 11:23 PM, Sergei Golubchik wrote:
> > > Okay, I understand what you did. But I don't understand why.
> > > What was the problem? Why there was a crash?
> >
> > - Item_func_make_set::val_str() returns &my_empty_string
> ...
> > After the fix my_empty_string:
> > - remembers that it is null terminated and does not call realloc() from
> >     c_ptr_safe()
> > - remembers that it is read-only and protects itself from realloc() by
> >     DBUG_ASSERT().
>
> I'd rather fix Item_func_make_set instead. Now it seems to be the only
> Item that uses it (there's also SELECT ... OUTFILE, but that's
> different).
>
> And Item_func_make_set has String tmp_str property, which is already
> used as an internal storage.
>
> So, I'd remove my_empty_string usage from Item_func_make_set whatsoever.
> Simply with
>
> -   String *result=&my_empty_string;
> +   String *result=&tmp_str;
> +   tmp_str.length(0);
>
> In fact, perhaps Item_func_make_set::tmp_str is not needed at all, and
> Item:str_value can be used instead:
>
> -   String *result=&my_empty_string;
> +   String *result=make_empty_result();

These types of bug are annoying. We've fixed a lot of them.
I hoped to extend the new code gradually for non-static strings as well:

- to fix String::c_ptr*() not to use realloc if constructor knows that
  the value is null terminated.
- protect use of c_ptr*() is unsafe contexts.

and also put String::thread_specific into String::flags when
merging to 10.0.

But this all can be done in 10.0 only. No need to do it in 5.3.


I will fix 5.3 simply to use make_empty_result().

> Regards,
> Sergei