Re: [patch/rfa] Tiny but delicate change to string::_M_mutate

[Nathan Myers <ncm-nospam at cantrip dot org>]

On Fri, Oct 15, 2004 at 08:26:18PM +0200, Paolo Carlini wrote:
> Hi everyone, hi Nathan,

Apologies for the delay in replying...

> I like a lot this tweak but I'm not going to apply it if Nathan is not
> able to double check it: is very small but delicate.
> 
> Basically, when Nathan improved last year the _S_empty_rep treatment
> (not ref-counted anymore) he added another case in the conditional
> at the beginning of _M_mutate:
> 
>    if (_M_rep() == &_S_empty_rep || __new_size > capacity() ...
> 
> Therefore, what change is that we reallocate even when __new_size ==
> capacity(). However, that seems not necessary, since, in that case,
> we have simply another empty string (for the empty_rep, capacity() == 0,
> of course). If we remove the new case, nothing happens in _M_mutate,
> besides eventually reconfirming the sharable state for the empty string
> object, length == 0 and _Rep::_S_terminal in position zero, the default
> contents, in other terms.

This looks perfectly safe.  It seems probable that people often do
append no characters, e.g. because the string they are appending
is empty.  Probably the logic should still check for _S_empty_rep
to avoid touching its reference count:

     if ((((_M_rep() == &_S_empty_rep() || _M_rep()->_M_is_shared()) 
            && (__len2 | __len1) != 0) 
	  || __new_size > capacity())

We might instead check that the proposed mutation actually changes 
the string before calling _M_mutate; that would subsume the case of 
appending the empty string, and make the present logic correct   
(...if I understand correctly).

Nathan Myers
ncm-nospam@cantrip.org