From b1801e0eb80854c1b96950de5c7082ec02346798 Mon Sep 17 00:00:00 2001 From: Gilles Depeyrot Date: Wed, 8 May 2002 14:11:40 +0000 Subject: [PATCH] improved out of memory handling by return failure status where possible and adding asserts in debug mode (suggested by SourceForge patch # 544906) corrected warnings when compiling with -Wall -W git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@15432 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/string.h | 62 +++++++----- src/common/string.cpp | 225 +++++++++++++++++++++++++++++------------- 2 files changed, 194 insertions(+), 93 deletions(-) diff --git a/include/wx/string.h b/include/wx/string.h index e36f11b7ec..af6c1ab17f 100644 --- a/include/wx/string.h +++ b/include/wx/string.h @@ -79,7 +79,7 @@ // implementation only #define wxASSERT_VALID_INDEX(i) \ - wxASSERT_MSG( (size_t)(i) <= Len(), _T("invaid index in wxString") ) + wxASSERT_MSG( (size_t)(i) <= Len(), _T("invalid index in wxString") ) // ---------------------------------------------------------------------------- // constants @@ -259,20 +259,20 @@ private: void Reinit() { GetStringData()->Unlock(); Init(); } // memory allocation - // allocates memory for string of lenght nLen - void AllocBuffer(size_t nLen); + // allocates memory for string of length nLen + bool AllocBuffer(size_t nLen); // copies data to another string - void AllocCopy(wxString&, int, int) const; + bool AllocCopy(wxString&, int, int) const; // effectively copies data to string - void AssignCopy(size_t, const wxChar *); + bool AssignCopy(size_t, const wxChar *); // append a (sub)string - void ConcatSelf(int nLen, const wxChar *src); + bool ConcatSelf(int nLen, const wxChar *src); // functions called before writing to the string: they copy it if there // are other references to our data (should be the only owner when writing) - void CopyBeforeWrite(); - void AllocBeforeWrite(size_t); + bool CopyBeforeWrite(); + bool AllocBeforeWrite(size_t); // if we hadn't made these operators private, it would be possible to // compile "wxString s; s = 17;" without any warnings as 17 is implicitly @@ -292,9 +292,9 @@ private: public: // constructors and destructor // ctor for an empty string - wxString() { Init(); } + wxString() : m_pchData(NULL) { Init(); } // copy ctor - wxString(const wxString& stringSrc) + wxString(const wxString& stringSrc) : m_pchData(NULL) { wxASSERT_MSG( stringSrc.GetStringData()->IsValid(), _T("did you forget to call UngetWriteBuf()?") ); @@ -313,9 +313,11 @@ public: // ctor takes first nLength characters from C string // (default value of wxSTRING_MAXLEN means take all the string) wxString(const wxChar *psz, size_t nLength = wxSTRING_MAXLEN) - { InitWith(psz, 0, nLength); } + : m_pchData(NULL) + { InitWith(psz, 0, nLength); } wxString(const wxChar *psz, wxMBConv& WXUNUSED(conv), size_t nLength = wxSTRING_MAXLEN) - { InitWith(psz, 0, nLength); } + : m_pchData(NULL) + { InitWith(psz, 0, nLength); } #if wxUSE_UNICODE // from multibyte string @@ -328,7 +330,8 @@ public: #else // ANSI // from C string (for compilers using unsigned char) wxString(const unsigned char* psz, size_t nLength = wxSTRING_MAXLEN) - { InitWith((const char*)psz, 0, nLength); } + : m_pchData(NULL) + { InitWith((const char*)psz, 0, nLength); } #if wxUSE_WCHAR_T // from wide (Unicode) string @@ -337,7 +340,8 @@ public: // from wxCharBuffer wxString(const wxCharBuffer& psz) - { InitWith(psz, 0, wxSTRING_MAXLEN); } + : m_pchData(NULL) + { InitWith(psz, 0, wxSTRING_MAXLEN); } #endif // Unicode/ANSI // dtor is not virtual, this class must not be inherited from! @@ -508,7 +512,8 @@ public: wxString& operator=(const wxChar *psz); #if wxUSE_UNICODE // from wxWCharBuffer - wxString& operator=(const wxWCharBuffer& psz) { return operator=((const wchar_t *)psz); } + wxString& operator=(const wxWCharBuffer& psz) + { (void) operator=((const wchar_t *)psz); return *this; } #else // ANSI // from another kind of C string wxString& operator=(const unsigned char* psz); @@ -517,7 +522,8 @@ public: wxString& operator=(const wchar_t *pwz); #endif // from wxCharBuffer - wxString& operator=(const wxCharBuffer& psz) { return operator=((const char *)psz); } + wxString& operator=(const wxCharBuffer& psz) + { (void) operator=((const char *)psz); return *this; } #endif // Unicode/ANSI // string concatenation @@ -724,10 +730,10 @@ public: // raw access to string memory // ensure that string has space for at least nLen characters // only works if the data of this string is not shared - void Alloc(size_t nLen); + bool Alloc(size_t nLen); // minimize the string's memory // only works if the data of this string is not shared - void Shrink(); + bool Shrink(); // get writable buffer of at least nLen bytes. Unget() *must* be called // a.s.a.p. to put string back in a reasonable state! wxChar *GetWriteBuf(size_t nLen); @@ -795,6 +801,7 @@ public: // constructors // take nLen chars starting at nPos wxString(const wxString& str, size_t nPos, size_t nLen) + : m_pchData(NULL) { wxASSERT_MSG( str.GetStringData()->IsValid(), _T("did you forget to call UngetWriteBuf()?") ); @@ -1005,7 +1012,9 @@ public: // constructors and destructor // default ctor - wxArrayString() { Init(FALSE); } + wxArrayString() + : m_nSize(0), m_nCount(0), m_pItems(NULL), m_autoSort(FALSE) + { Init(FALSE); } // if autoSort is TRUE, the array is always sorted (in alphabetical order) // // NB: the reason for using int and not bool is that like this we can avoid @@ -1014,7 +1023,9 @@ public: // // of course, using explicit would be even better - if all compilers // supported it... - wxArrayString(int autoSort) { Init(autoSort != 0); } + wxArrayString(int autoSort) + : m_nSize(0), m_nCount(0), m_pItems(NULL), m_autoSort(FALSE) + { Init(autoSort != 0); } // copy ctor wxArrayString(const wxArrayString& array); // assignment operator @@ -1131,14 +1142,17 @@ public: class WXDLLEXPORT wxStringBuffer { + DECLARE_NO_COPY_CLASS(wxStringBuffer) + public: wxStringBuffer(wxString& str, size_t lenWanted = 1024) - : m_str(str) { m_buf = m_str.GetWriteBuf(lenWanted); } - + : m_str(str), m_buf(NULL) + { m_buf = m_str.GetWriteBuf(lenWanted); } + ~wxStringBuffer() { m_str.UngetWriteBuf(); } - + operator wxChar*() const { return m_buf; } - + private: wxString& m_str; wxChar *m_buf; diff --git a/src/common/string.cpp b/src/common/string.cpp index ea40c4d411..e232c8a79d 100644 --- a/src/common/string.cpp +++ b/src/common/string.cpp @@ -267,7 +267,10 @@ wxString::wxString(wxChar ch, size_t nLength) Init(); if ( nLength > 0 ) { - AllocBuffer(nLength); + if ( !AllocBuffer(nLength) ) { + wxFAIL_MSG( _T("out of memory in wxString::wxString") ); + return; + } #if wxUSE_UNICODE // memset only works on char @@ -294,7 +297,10 @@ void wxString::InitWith(const wxChar *psz, size_t nPos, size_t nLength) if ( nLength > 0 ) { // trailing '\0' is written in AllocBuffer() - AllocBuffer(nLength); + if ( !AllocBuffer(nLength) ) { + wxFAIL_MSG( _T("out of memory in wxString::InitWith") ); + return; + } memcpy(m_pchData, psz + nPos, nLength*sizeof(wxChar)); } } @@ -324,7 +330,10 @@ wxString::wxString(const char *psz, wxMBConv& conv, size_t nLength) // empty? if ( (nLen != 0) && (nLen != (size_t)-1) ) { - AllocBuffer(nLen); + if ( !AllocBuffer(nLen) ) { + wxFAIL_MSG( _T("out of memory in wxString::wxString") ); + return; + } conv.MB2WC(m_pchData, psz, nLen); } else { @@ -350,7 +359,10 @@ wxString::wxString(const wchar_t *pwz, wxMBConv& conv, size_t nLength) // empty? if ( (nLen != 0) && (nLen != (size_t)-1) ) { - AllocBuffer(nLen); + if ( !AllocBuffer(nLen) ) { + wxFAIL_MSG( _T("out of memory in wxString::wxString") ); + return; + } conv.WC2MB(m_pchData, pwz, nLen); } else { @@ -366,7 +378,7 @@ wxString::wxString(const wchar_t *pwz, wxMBConv& conv, size_t nLength) // --------------------------------------------------------------------------- // allocates memory needed to store a C string of length nLen -void wxString::AllocBuffer(size_t nLen) +bool wxString::AllocBuffer(size_t nLen) { // allocating 0 sized buffer doesn't make sense, all empty strings should // reuse g_strEmpty @@ -383,30 +395,42 @@ void wxString::AllocBuffer(size_t nLen) // 2) sizeof(wxStringData) for housekeeping info wxStringData* pData = (wxStringData*) malloc(sizeof(wxStringData) + (nLen + EXTRA_ALLOC + 1)*sizeof(wxChar)); + + if ( pData == NULL ) { + // allocation failures are handled by the caller + return FALSE; + } + pData->nRefs = 1; pData->nDataLength = nLen; pData->nAllocLength = nLen + EXTRA_ALLOC; m_pchData = pData->data(); // data starts after wxStringData m_pchData[nLen] = wxT('\0'); + return TRUE; } // must be called before changing this string -void wxString::CopyBeforeWrite() +bool wxString::CopyBeforeWrite() { wxStringData* pData = GetStringData(); if ( pData->IsShared() ) { pData->Unlock(); // memory not freed because shared size_t nLen = pData->nDataLength; - AllocBuffer(nLen); + if ( !AllocBuffer(nLen) ) { + // allocation failures are handled by the caller + return FALSE; + } memcpy(m_pchData, pData->data(), nLen*sizeof(wxChar)); } wxASSERT( !GetStringData()->IsShared() ); // we must be the only owner + + return TRUE; } // must be called before replacing contents of this string -void wxString::AllocBeforeWrite(size_t nLen) +bool wxString::AllocBeforeWrite(size_t nLen) { wxASSERT( nLen != 0 ); // doesn't make any sense @@ -415,7 +439,10 @@ void wxString::AllocBeforeWrite(size_t nLen) if ( pData->IsShared() || pData->IsEmpty() ) { // can't work with old buffer, get new one pData->Unlock(); - AllocBuffer(nLen); + if ( !AllocBuffer(nLen) ) { + // allocation failures are handled by the caller + return FALSE; + } } else { if ( nLen > pData->nAllocLength ) { @@ -425,15 +452,13 @@ void wxString::AllocBeforeWrite(size_t nLen) nLen += EXTRA_ALLOC; - wxStringData *pDataOld = pData; pData = (wxStringData*) realloc(pData, sizeof(wxStringData) + (nLen + 1)*sizeof(wxChar)); - if ( !pData ) { - // out of memory - free(pDataOld); - - // FIXME we're going to crash... - return; + + if ( pData == NULL ) { + // allocation failures are handled by the caller + // keep previous data since reallocation failed + return FALSE; } pData->nAllocLength = nLen; @@ -445,10 +470,12 @@ void wxString::AllocBeforeWrite(size_t nLen) } wxASSERT( !GetStringData()->IsShared() ); // we must be the only owner + + return TRUE; } // allocate enough memory for nLen characters -void wxString::Alloc(size_t nLen) +bool wxString::Alloc(size_t nLen) { wxStringData *pData = GetStringData(); if ( pData->nAllocLength <= nLen ) { @@ -456,7 +483,13 @@ void wxString::Alloc(size_t nLen) nLen += EXTRA_ALLOC; wxStringData* pData = (wxStringData*) - malloc(sizeof(wxStringData) + (nLen + 1)*sizeof(wxChar)); + malloc(sizeof(wxStringData) + (nLen + 1)*sizeof(wxChar)); + + if ( pData == NULL ) { + // allocation failure handled by caller + return FALSE; + } + pData->nRefs = 1; pData->nDataLength = 0; pData->nAllocLength = nLen; @@ -466,42 +499,47 @@ void wxString::Alloc(size_t nLen) else if ( pData->IsShared() ) { pData->Unlock(); // memory not freed because shared size_t nOldLen = pData->nDataLength; - AllocBuffer(nLen); + if ( !AllocBuffer(nLen) ) { + // allocation failure handled by caller + return FALSE; + } memcpy(m_pchData, pData->data(), nOldLen*sizeof(wxChar)); } else { nLen += EXTRA_ALLOC; - wxStringData *pDataOld = pData; - wxStringData *p = (wxStringData *) + pData = (wxStringData *) realloc(pData, sizeof(wxStringData) + (nLen + 1)*sizeof(wxChar)); - if ( p == NULL ) { - // don't leak memory - free(pDataOld); - - // FIXME what to do on memory error? - return; + if ( pData == NULL ) { + // allocation failure handled by caller + // keep previous data since reallocation failed + return FALSE; } // it's not important if the pointer changed or not (the check for this // is not faster than assigning to m_pchData in all cases) - p->nAllocLength = nLen; - m_pchData = p->data(); + pData->nAllocLength = nLen; + m_pchData = pData->data(); } } //else: we've already got enough + return TRUE; } // shrink to minimal size (releasing extra memory) -void wxString::Shrink() +bool wxString::Shrink() { wxStringData *pData = GetStringData(); size_t nLen = pData->nDataLength; void *p = realloc(pData, sizeof(wxStringData) + (nLen + 1)*sizeof(wxChar)); - wxASSERT_MSG( p != NULL, _T("can't free memory?") ); + if ( p == NULL) { + wxFAIL_MSG( _T("out of memory reallocating wxString data") ); + // keep previous data since reallocation failed + return FALSE; + } if ( p != pData ) { @@ -513,12 +551,17 @@ void wxString::Shrink() } pData->nAllocLength = nLen; + + return TRUE; } // get the pointer to writable buffer of (at least) nLen bytes wxChar *wxString::GetWriteBuf(size_t nLen) { - AllocBeforeWrite(nLen); + if ( !AllocBeforeWrite(nLen) ) { + // allocation failure handled by caller + return NULL; + } wxASSERT( GetStringData()->nRefs == 1 ); GetStringData()->Validate(FALSE); @@ -550,17 +593,21 @@ void wxString::UngetWriteBuf(size_t nLen) // --------------------------------------------------------------------------- // helper function: does real copy -void wxString::AssignCopy(size_t nSrcLen, const wxChar *pszSrcData) +bool wxString::AssignCopy(size_t nSrcLen, const wxChar *pszSrcData) { if ( nSrcLen == 0 ) { Reinit(); } else { - AllocBeforeWrite(nSrcLen); + if ( !AllocBeforeWrite(nSrcLen) ) { + // allocation failure handled by caller + return FALSE; + } memcpy(m_pchData, pszSrcData, nSrcLen*sizeof(wxChar)); GetStringData()->nDataLength = nSrcLen; m_pchData[nSrcLen] = wxT('\0'); } + return TRUE; } // assigns one string to another @@ -587,7 +634,9 @@ wxString& wxString::operator=(const wxString& stringSrc) // assigns a single character wxString& wxString::operator=(wxChar ch) { - AssignCopy(1, &ch); + if ( !AssignCopy(1, &ch) ) { + wxFAIL_MSG( _T("out of memory in wxString::operator=(wxChar)") ); + } return *this; } @@ -595,7 +644,9 @@ wxString& wxString::operator=(wxChar ch) // assigns C string wxString& wxString::operator=(const wxChar *psz) { - AssignCopy(wxStrlen(psz), psz); + if ( !AssignCopy(wxStrlen(psz), psz) ) { + wxFAIL_MSG( _T("out of memory in wxString::operator=(const wxChar *)") ); + } return *this; } @@ -624,7 +675,7 @@ wxString& wxString::operator=(const wchar_t *pwz) // --------------------------------------------------------------------------- // add something to this string -void wxString::ConcatSelf(int nSrcLen, const wxChar *pszSrcData) +bool wxString::ConcatSelf(int nSrcLen, const wxChar *pszSrcData) { STATISTICS_ADD(SummandLength, nSrcLen); @@ -640,7 +691,10 @@ void wxString::ConcatSelf(int nSrcLen, const wxChar *pszSrcData) // we have to allocate another buffer wxStringData* pOldData = GetStringData(); - AllocBuffer(nNewLen); + if ( !AllocBuffer(nNewLen) ) { + // allocation failure handled by caller + return FALSE; + } memcpy(m_pchData, pOldData->data(), nLen*sizeof(wxChar)); pOldData->Unlock(); } @@ -648,7 +702,10 @@ void wxString::ConcatSelf(int nSrcLen, const wxChar *pszSrcData) STATISTICS_ADD(ConcatHit, 0); // we have to grow the buffer - Alloc(nNewLen); + if ( !Alloc(nNewLen) ) { + // allocation failure handled by caller + return FALSE; + } } else { STATISTICS_ADD(ConcatHit, 1); @@ -666,6 +723,7 @@ void wxString::ConcatSelf(int nSrcLen, const wxChar *pszSrcData) GetStringData()->nDataLength = nNewLen; // and fix the length } //else: the string to append was empty + return TRUE; } /* @@ -675,57 +733,61 @@ void wxString::ConcatSelf(int nSrcLen, const wxChar *pszSrcData) * C str + string and string + C str */ -wxString operator+(const wxString& string1, const wxString& string2) +wxString operator+(const wxString& str1, const wxString& str2) { - wxASSERT( string1.GetStringData()->IsValid() ); - wxASSERT( string2.GetStringData()->IsValid() ); + wxASSERT( str1.GetStringData()->IsValid() ); + wxASSERT( str2.GetStringData()->IsValid() ); - wxString s = string1; - s += string2; + wxString s = str1; + s += str2; return s; } -wxString operator+(const wxString& string, wxChar ch) +wxString operator+(const wxString& str, wxChar ch) { - wxASSERT( string.GetStringData()->IsValid() ); + wxASSERT( str.GetStringData()->IsValid() ); - wxString s = string; + wxString s = str; s += ch; return s; } -wxString operator+(wxChar ch, const wxString& string) +wxString operator+(wxChar ch, const wxString& str) { - wxASSERT( string.GetStringData()->IsValid() ); + wxASSERT( str.GetStringData()->IsValid() ); wxString s = ch; - s += string; + s += str; return s; } -wxString operator+(const wxString& string, const wxChar *psz) +wxString operator+(const wxString& str, const wxChar *psz) { - wxASSERT( string.GetStringData()->IsValid() ); + wxASSERT( str.GetStringData()->IsValid() ); wxString s; - s.Alloc(wxStrlen(psz) + string.Len()); - s = string; + if ( !s.Alloc(wxStrlen(psz) + str.Len()) ) { + wxFAIL_MSG( _T("out of memory in wxString::operator+") ); + } + s = str; s += psz; return s; } -wxString operator+(const wxChar *psz, const wxString& string) +wxString operator+(const wxChar *psz, const wxString& str) { - wxASSERT( string.GetStringData()->IsValid() ); + wxASSERT( str.GetStringData()->IsValid() ); wxString s; - s.Alloc(wxStrlen(psz) + string.Len()); + if ( !s.Alloc(wxStrlen(psz) + str.Len()) ) { + wxFAIL_MSG( _T("out of memory in wxString::operator+") ); + } s = psz; - s += string; + s += str; return s; } @@ -739,15 +801,19 @@ wxString operator+(const wxChar *psz, const wxString& string) // --------------------------------------------------------------------------- // helper function: clone the data attached to this string -void wxString::AllocCopy(wxString& dest, int nCopyLen, int nCopyIndex) const +bool wxString::AllocCopy(wxString& dest, int nCopyLen, int nCopyIndex) const { if ( nCopyLen == 0 ) { dest.Init(); } else { - dest.AllocBuffer(nCopyLen); + if ( !dest.AllocBuffer(nCopyLen) ) { + // allocation failure handled by caller + return FALSE; + } memcpy(dest.m_pchData, m_pchData + nCopyIndex, nCopyLen*sizeof(wxChar)); } + return TRUE; } // extract string of length nCount starting at nFirst @@ -775,7 +841,9 @@ wxString wxString::Mid(size_t nFirst, size_t nCount) const } wxString dest; - AllocCopy(dest, nCount, nFirst); + if ( !AllocCopy(dest, nCount, nFirst) ) { + wxFAIL_MSG( _T("out of memory in wxString::Mid") ); + } return dest; } @@ -816,7 +884,9 @@ wxString wxString::Right(size_t nCount) const nCount = GetStringData()->nDataLength; wxString dest; - AllocCopy(dest, nCount, GetStringData()->nDataLength - nCount); + if ( !AllocCopy(dest, nCount, GetStringData()->nDataLength - nCount) ) { + wxFAIL_MSG( _T("out of memory in wxString::Right") ); + } return dest; } @@ -841,7 +911,9 @@ wxString wxString::Left(size_t nCount) const nCount = GetStringData()->nDataLength; wxString dest; - AllocCopy(dest, nCount, 0); + if ( !AllocCopy(dest, nCount, 0) ) { + wxFAIL_MSG( _T("out of memory in wxString::Left") ); + } return dest; } @@ -902,7 +974,10 @@ size_t wxString::Replace(const wxChar *szOld, const wxChar *szNew, bool bReplace } else { // take chars before match - strTemp.ConcatSelf(pSubstr - pCurrent, pCurrent); + if ( !strTemp.ConcatSelf(pSubstr - pCurrent, pCurrent) ) { + wxFAIL_MSG( _T("out of memory in wxString::Replace") ); + return 0; + } strTemp += szNew; pCurrent = pSubstr + uiOldLen; // restart after match @@ -968,8 +1043,11 @@ wxString wxString::Strip(stripType w) const wxString& wxString::MakeUpper() { - CopyBeforeWrite(); - + if ( !CopyBeforeWrite() ) { + wxFAIL_MSG( _T("out of memory in wxString::MakeUpper") ); + return *this; + } + for ( wxChar *p = m_pchData; *p; p++ ) *p = (wxChar)wxToupper(*p); @@ -978,7 +1056,10 @@ wxString& wxString::MakeUpper() wxString& wxString::MakeLower() { - CopyBeforeWrite(); + if ( !CopyBeforeWrite() ) { + wxFAIL_MSG( _T("out of memory in wxString::MakeLower") ); + return *this; + } for ( wxChar *p = m_pchData; *p; p++ ) *p = (wxChar)wxTolower(*p); @@ -1010,7 +1091,10 @@ wxString& wxString::Trim(bool bFromRight) ) { // ok, there is at least one space to trim - CopyBeforeWrite(); + if ( !CopyBeforeWrite() ) { + wxFAIL_MSG( _T("out of memory in wxString::Trim") ); + return *this; + } if ( bFromRight ) { @@ -1060,7 +1144,10 @@ wxString& wxString::Pad(size_t nCount, wxChar chPad, bool bFromRight) wxString& wxString::Truncate(size_t uiLen) { if ( uiLen < Len() ) { - CopyBeforeWrite(); + if ( !CopyBeforeWrite() ) { + wxFAIL_MSG( _T("out of memory in wxString::Truncate") ); + return *this; + } *(m_pchData + uiLen) = wxT('\0'); GetStringData()->nDataLength = uiLen;