[PATCH] don't store temporary value returned from c_str()

Tomi Ollila tomi.ollila at iki.fi
Sat Apr 27 06:22:01 PDT 2013


On Sat, Apr 27 2013, Jani Nikula <jani at nikula.org> wrote:

> On Sat, 20 Apr 2013, Vladimir.Marek at oracle.com wrote:
>> From: Vladimir Marek <vlmarek at volny.cz>
>>
>> This is causing problems when compiled by Oracle Studio. Memory pointed
>> by (const char*)term was already changed once talloc_strdup was called.
>>
>> Signed-off-by: Vladimir Marek <vlmarek at volny.cz>
>> ---
>>  lib/message.cc |    9 ++++-----
>>  1 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/message.cc b/lib/message.cc
>> index 8720c1b..8d329d1 100644
>> --- a/lib/message.cc
>> +++ b/lib/message.cc
>> @@ -266,18 +266,17 @@ _notmuch_message_get_term (notmuch_message_t *message,
>>  			   const char *prefix)
>>  {
>>      int prefix_len = strlen (prefix);
>> -    const char *term = NULL;
>>      char *value;
>>  
>>      i.skip_to (prefix);
>>  
>> -    if (i != end)
>> -	term = (*i).c_str ();
>
> It's okay to use the result of .c_str() as long as the string object
> stays in scope, and none of the non-const member functions are
> called. Here, I think the problem is that TermIterator's overloaded
> operator*() returns a string object within the if block's scope, and it
> goes immediately out of scope. You could check this by adding
>
> 	string s = *i;
>
> in function scope, and replacing (*i) with s in the if block. This might
> also be more obvious than the presented patch, but I think the patch is
> fine too.

Thanks, now I understand. I'd like to see updated patch using Jani's
example but I also think that the current patch is fine too.

>
> BR,
> Jani.

Tomi

>
>
>> +    if (i == end)
>> +	return NULL;
>>  
>> -    if (!term || strncmp (term, prefix, prefix_len))
>> +    if (strncmp ((*i).c_str(), prefix, prefix_len))
>>  	return NULL;
>>  
>> -    value = talloc_strdup (message, term + prefix_len);
>> +    value = talloc_strdup (message, (*i).c_str() + prefix_len);
>>  
>>  #if DEBUG_DATABASE_SANITY
>>      i++;
>> -- 
>> 1.7.3.2
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch at notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


More information about the notmuch mailing list