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