From: Michael Widenius
Date: October 14 2009 8:20pm
Subject: Re: String:c_ptr makes me unhappy
List-Archive: http://lists.mysql.com/internals/37374
Message-Id: <19158.12948.720225.949847@narttu.askmonty.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit


Hi!

>>>>> "MARK" == MARK CALLAGHAN <mdcallag@stripped> writes:

MARK> On Wed, Oct 14, 2009 at 9:13 AM, Michael Widenius <monty@stripped> wrote:
>> 
>> Hi!
>> 
>>>>>>> "MARK" == MARK CALLAGHAN <mdcallag@stripped> writes:

>> 
MARK> 2) Change the name of c_ptr as it doesn't return a C pointer to a
MARK> string. If it did, the result would be nul terminated and there would
MARK> be no need for c_ptr_unsafe.
>> 
>> c_ptr does return a pointer to a \0 terminated string. It does however
>> don't know if the last byte is initialized memory or not, which is a
>> problem for valgrind but not normally for MySQL as Strings is mainly
>> used for thread specific memory.

MARK> If you use it in that way then this code has a race. When
MARK> Ptr[str_length] == 0, it doesn't realloc and nul-terminate the string.
MARK> But Ptr[str_length] might not have been allocated for this string. So
MARK> it Ptr[str_length] might be 0 when the check is made and then not 0
MARK> when this returns.

As we only use c_ptr() for things that is to be used straight away and
it's in thread specific memory (ie, will not be changed by other
processes), this should not be a problem.

One common reason for the current code is to allow String's to be used
as:

String *A= String("hello",5);
char *b= A->c_ptr();

In this case, it's totally safe to check the end \0 and not realloc
the string.  Without the current code, we would have to realloc all
similar strings always when using c_ptr().

MARK>   inline char *c_ptr()
MARK>   {
MARK>     if (!Ptr || Ptr[str_length])                /* Should be safe */
MARK>       (void) realloc(str_length);
MARK>     return Ptr;
MARK>   }

As far as I can remember, I haven't seen very, very few bugs in the
way String is used in MySQL becasue of c_ptr().

I do however agree that there has been several cases where we had to
replace c_ptr() with c_ptr_safe() just to get rid of valgrind warnings.

Regards,
Monty