From d545bdede67c1f4335e6b4822dfcad3775f631fc Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 21 Aug 2007 16:45:41 +0000 Subject: [PATCH] Fix bug with assigning a part of the string to the same string The fix has several parts: 1. don't free the old string data in ConcatSelf() if we use it as source 2. implement assign() using replace() rather than clear() + append() 3. fix replace() to work with replacement strings containing embedded NULs and optimize it by using memcpy() instead of byte-wise copy git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@48302 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/stringimpl.h | 23 ++++++---- src/common/stringimpl.cpp | 92 ++++++++++++++++++++------------------- 2 files changed, 61 insertions(+), 54 deletions(-) diff --git a/include/wx/stringimpl.h b/include/wx/stringimpl.h index c7029974c9..7ac70a6c5e 100644 --- a/include/wx/stringimpl.h +++ b/include/wx/stringimpl.h @@ -416,18 +416,18 @@ public: { return *this = str; } // same as ` = str[pos..pos + n] wxStringImpl& assign(const wxStringImpl& str, size_t pos, size_t n) - { clear(); return append(str, pos, n); } + { return replace(0, npos, str, pos, n); } // same as `= first n (or all if n == npos) characters of sz' wxStringImpl& assign(const wxStringCharType *sz) - { clear(); return append(sz, wxStrlen(sz)); } + { return replace(0, npos, sz, wxStrlen(sz)); } wxStringImpl& assign(const wxStringCharType *sz, size_t n) - { clear(); return append(sz, n); } + { return replace(0, npos, sz, n); } // same as `= n copies of ch' wxStringImpl& assign(size_t n, wxStringCharType ch) - { clear(); return append(n, ch); } + { return replace(0, npos, n, ch); } // assign from first to last wxStringImpl& assign(const_iterator first, const_iterator last) - { clear(); return append(first, last); } + { return replace(begin(), end(), first, last); } // first valid index position const_iterator begin() const { return m_pchData; } @@ -478,18 +478,23 @@ public: const wxStringCharType* data() const { return m_pchData; } // replaces the substring of length nLen starting at nStart - wxStringImpl& replace(size_t nStart, size_t nLen, const wxStringCharType* sz); + wxStringImpl& replace(size_t nStart, size_t nLen, const wxStringCharType* sz) + { return replace(nStart, nLen, sz, npos); } // replaces the substring of length nLen starting at nStart wxStringImpl& replace(size_t nStart, size_t nLen, const wxStringImpl& str) - { return replace(nStart, nLen, str.c_str()); } + { return replace(nStart, nLen, str.c_str(), str.length()); } // replaces the substring with nCount copies of ch - wxStringImpl& replace(size_t nStart, size_t nLen, size_t nCount, wxStringCharType ch); + wxStringImpl& replace(size_t nStart, size_t nLen, + size_t nCount, wxStringCharType ch) + { return replace(nStart, nLen, wxStringImpl(nCount, ch)); } // replaces a substring with another substring wxStringImpl& replace(size_t nStart, size_t nLen, - const wxStringImpl& str, size_t nStart2, size_t nLen2); + const wxStringImpl& str, size_t nStart2, size_t nLen2) + { return replace(nStart, nLen, str.substr(nStart2, nLen2)); } // replaces the substring with first nCount chars of sz wxStringImpl& replace(size_t nStart, size_t nLen, const wxStringCharType* sz, size_t nCount); + wxStringImpl& replace(iterator first, iterator last, const_pointer s) { return replace(first - begin(), last - first, s); } wxStringImpl& replace(iterator first, iterator last, const_pointer s, diff --git a/src/common/stringimpl.cpp b/src/common/stringimpl.cpp index af847b54e1..95d6863301 100644 --- a/src/common/stringimpl.cpp +++ b/src/common/stringimpl.cpp @@ -576,54 +576,45 @@ size_t wxStringImpl::rfind(wxStringCharType ch, size_t nStart) const } wxStringImpl& wxStringImpl::replace(size_t nStart, size_t nLen, - const wxStringCharType *sz) -{ - wxASSERT_MSG( nStart <= length(), - _T("index out of bounds in wxStringImpl::replace") ); - size_t strLen = length() - nStart; - nLen = strLen < nLen ? strLen : nLen; - - wxStringImpl strTmp; - strTmp.reserve(length()); // micro optimisation to avoid multiple mem allocs - - //This is kind of inefficient, but its pretty good considering... - //we don't want to use character access operators here because on STL - //it will freeze the reference count of strTmp, which means a deep copy - //at the end when swap is called - // - //Also, we can't use append with the full character pointer and must - //do it manually because this string can contain null characters - for(size_t i1 = 0; i1 < nStart; ++i1) - strTmp.append(1, this->c_str()[i1]); - - //its safe to do the full version here because - //sz must be a normal c string - strTmp.append(sz); - - for(size_t i2 = nStart + nLen; i2 < length(); ++i2) - strTmp.append(1, this->c_str()[i2]); - - swap(strTmp); - return *this; -} - -wxStringImpl& wxStringImpl::replace(size_t nStart, size_t nLen, - size_t nCount, wxStringCharType ch) + const wxStringCharType *sz, size_t nCount) { - return replace(nStart, nLen, wxStringImpl(nCount, ch).c_str()); -} + // check and adjust parameters + const size_t lenOld = length(); -wxStringImpl& wxStringImpl::replace(size_t nStart, size_t nLen, - const wxStringImpl& str, - size_t nStart2, size_t nLen2) -{ - return replace(nStart, nLen, str.substr(nStart2, nLen2)); -} + wxASSERT_MSG( nStart <= lenOld, + _T("index out of bounds in wxStringImpl::replace") ); + size_t nEnd = nStart + nLen; + if ( nEnd > lenOld ) + { + // nLen may be out of range, as it can be npos, just clump it down + nLen = lenOld - nStart; + nEnd = lenOld; + } -wxStringImpl& wxStringImpl::replace(size_t nStart, size_t nLen, - const wxStringCharType* sz, size_t nCount) -{ - return replace(nStart, nLen, wxStringImpl(sz, nCount).c_str()); + if ( nCount == npos ) + nCount = wxStrlen(sz); + + // build the new string from 3 pieces: part of this string before nStart, + // the new substring and the part of this string after nStart+nLen + wxStringImpl tmp; + const size_t lenNew = lenOld + nCount - nLen; + if ( lenNew ) + { + tmp.AllocBuffer(lenOld + nCount - nLen); + + wxStringCharType *dst = tmp.m_pchData; + memcpy(dst, m_pchData, nStart*sizeof(wxStringCharType)); + dst += nStart; + + memcpy(dst, sz, nCount*sizeof(wxStringCharType)); + dst += nCount; + + memcpy(dst, m_pchData + nEnd, (lenOld - nEnd)*sizeof(wxStringCharType)); + } + + // and replace this string contents with the new one + swap(tmp); + return *this; } wxStringImpl wxStringImpl::substr(size_t nStart, size_t nLen) const @@ -709,6 +700,17 @@ bool wxStringImpl::ConcatSelf(size_t nSrcLen, if ( nSrcLen > 0 ) { wxStringData *pData = GetStringData(); size_t nLen = pData->nDataLength; + + // take special care when appending part of this string to itself: the code + // below reallocates our buffer and this invalidates pszSrcData pointer so + // we have to copy it in another temporary string in this case (but avoid + // doing this unnecessarily) + if ( pszSrcData >= m_pchData && pszSrcData < m_pchData + nLen ) + { + wxStringImpl tmp(pszSrcData, nSrcLen); + return ConcatSelf(nSrcLen, tmp.m_pchData, nSrcLen); + } + size_t nNewLen = nLen + nSrcLen; // alloc new buffer if current is too small -- 2.47.2