From c504eafdc88be761ec5a761607b1904ab2aebce0 Mon Sep 17 00:00:00 2001 From: Michael Ow Date: Mon, 9 Jun 2008 21:18:46 +0000 Subject: [PATCH] ICU-5955 Add freeOffsetBuffer function to make sure that previously allocated offsetBuffer is freed to avoid memory leaks. Add deleter method in ssearch.cpp to remove hashtable objects. Delete various objects in intltest after it is no longer needed. X-SVN-Rev: 24129 --- icu4c/source/i18n/coll.cpp | 2 +- icu4c/source/i18n/ucol.cpp | 14 +++++++++++++- icu4c/source/i18n/ucol_imp.h | 10 ++++++++++ icu4c/source/i18n/ucoleitr.cpp | 11 +++++++---- icu4c/source/i18n/usearch.cpp | 4 ++++ icu4c/source/test/intltest/ssearch.cpp | 16 ++++++++++++++-- icu4c/source/test/intltest/tzrulets.cpp | 1 + 7 files changed, 50 insertions(+), 8 deletions(-) diff --git a/icu4c/source/i18n/coll.cpp b/icu4c/source/i18n/coll.cpp index b0c177063f0..7329ef3f07b 100644 --- a/icu4c/source/i18n/coll.cpp +++ b/icu4c/source/i18n/coll.cpp @@ -349,7 +349,7 @@ Collator* Collator::makeInstance(const Locale& desiredLocale, // non-table-based Collator in some other way, when it sees that it needs // to. // The specific caution is this: RuleBasedCollator(Locale&) will ALWAYS - // return a valid collation object, if the system if functioning properly. + // return a valid collation object, if the system is functioning properly. // The reason is that it will fall back, use the default locale, and even // use the built-in default collation rules. THEREFORE, createInstance() // should in general ONLY CALL RuleBasedCollator(Locale&) IF IT KNOWS IN diff --git a/icu4c/source/i18n/ucol.cpp b/icu4c/source/i18n/ucol.cpp index 8ac16695cce..419e098d4b0 100644 --- a/icu4c/source/i18n/ucol.cpp +++ b/icu4c/source/i18n/ucol.cpp @@ -5467,6 +5467,9 @@ cleanup: uprv_free(caseStart); uprv_free(quadStart); } + + /* To avoid memory leak, free the offset buffer if necessary. */ + freeOffsetBuffer(&s); if(normSource != normBuffer) { uprv_free(normSource); @@ -5848,7 +5851,10 @@ cleanup: uprv_free(terStart); uprv_free(secStart); } - + + /* To avoid memory leak, free the offset buffer if necessary. */ + freeOffsetBuffer(&s); + if(normSource != normBuffer) { uprv_free(normSource); } @@ -6709,6 +6715,9 @@ saveState: unorm_closeIter(normIter); } + /* To avoid memory leak, free the offset buffer if necessary. */ + freeOffsetBuffer(&s); + // Return number of meaningful sortkey bytes. UTRACE_DATA4(UTRACE_VERBOSE, "dest = %vb, state=%d %d", dest,i, state[0], state[1]); @@ -7111,6 +7120,9 @@ ucol_setVariableTop(UCollator *coll, const UChar *varTop, int32_t len, UErrorCod coll->variableTopValueisDefault = FALSE; coll->variableTopValue = (CE & UCOL_PRIMARYMASK)>>16; } + + /* To avoid memory leak, free the offset buffer if necessary. */ + freeOffsetBuffer(&s); return CE & UCOL_PRIMARYMASK; } diff --git a/icu4c/source/i18n/ucol_imp.h b/icu4c/source/i18n/ucol_imp.h index b39ac444ae3..1ffd4eb0c4e 100644 --- a/icu4c/source/i18n/ucol_imp.h +++ b/icu4c/source/i18n/ucol_imp.h @@ -43,6 +43,7 @@ #include "unicode/ucol.h" #include "utrie.h" +#include "cmemory.h" /* This is the internal header file which contains important declarations for * the collation framework. @@ -1039,6 +1040,15 @@ static inline UBool ucol_unsafeCP(UChar c, const UCollator *coll) { } #endif /* XP_CPLUSPLUS */ +/* The offsetBuffer in collIterate might need to be freed to avoid memory leaks. */ +static inline void freeOffsetBuffer(collIterate *s) { + if (s != NULL && s->offsetBuffer != NULL) { + uprv_free(s->offsetBuffer); + s->offsetBuffer = NULL; + s->offsetBufferSize = 0; + } +} + #endif /* #if !UCONFIG_NO_COLLATION */ diff --git a/icu4c/source/i18n/ucoleitr.cpp b/icu4c/source/i18n/ucoleitr.cpp index 0666f69f9be..0b7751e4891 100644 --- a/icu4c/source/i18n/ucoleitr.cpp +++ b/icu4c/source/i18n/ucoleitr.cpp @@ -319,10 +319,10 @@ ucol_openElements(const UCollator *coll, *status = U_MEMORY_ALLOCATION_ERROR; return NULL; } - - result->reset_ = TRUE; - result->isWritable = FALSE; - result->pce = NULL; + + result->reset_ = TRUE; + result->isWritable = FALSE; + result->pce = NULL; if (text == NULL) { textLength = 0; @@ -672,6 +672,9 @@ ucol_setText( UCollationElements *elems, } elems->isWritable = FALSE; + + /* free offset buffer to avoid memory leak before initializing. */ + freeOffsetBuffer(&(elems->iteratordata_)); uprv_init_collIterate(elems->iteratordata_.coll, text, textLength, &elems->iteratordata_); diff --git a/icu4c/source/i18n/usearch.cpp b/icu4c/source/i18n/usearch.cpp index 486ccb440b3..f320f7c2ccf 100644 --- a/icu4c/source/i18n/usearch.cpp +++ b/icu4c/source/i18n/usearch.cpp @@ -3044,6 +3044,8 @@ U_CAPI void U_EXPORT2 usearch_setCollator( UStringSearch *strsrch, if (U_SUCCESS(*status)) { initialize(strsrch, status); if (U_SUCCESS(*status)) { + /* free offset buffer to avoid memory leak before initializing. */ + freeOffsetBuffer(&(strsrch->textIter->iteratordata_)); uprv_init_collIterate(collator, strsrch->search->text, strsrch->search->textLength, &(strsrch->textIter->iteratordata_)); @@ -3426,6 +3428,8 @@ U_CAPI void U_EXPORT2 usearch_reset(UStringSearch *strsrch) if (!sameCollAttribute) { initialize(strsrch, &status); } + /* free offset buffer to avoid memory leak before initializing. */ + freeOffsetBuffer(&(strsrch->textIter->iteratordata_)); uprv_init_collIterate(strsrch->collator, strsrch->search->text, strsrch->search->textLength, &(strsrch->textIter->iteratordata_)); diff --git a/icu4c/source/test/intltest/ssearch.cpp b/icu4c/source/test/intltest/ssearch.cpp index d3fec9347f5..0812e54c66e 100644 --- a/icu4c/source/test/intltest/ssearch.cpp +++ b/icu4c/source/test/intltest/ssearch.cpp @@ -641,7 +641,7 @@ void SSearchTest::offsetTest() backwardList.reverse(); if (forwardList.compare(backwardList)) { - logln("Works with \"%S\"", test[i].getTerminatedBuffer()); + logln("Works with \"%s\"", test[i].getTerminatedBuffer()); logln("Forward offsets: [%s]", printOffsets(buffer, forwardList)); // logln("Backward offsets: [%s]", printOffsets(buffer, backwardList)); @@ -659,7 +659,9 @@ void SSearchTest::offsetTest() infoln(); } + delete iter; } + delete col; } class CEList @@ -914,6 +916,7 @@ public: private: static void deleteCEList(void *obj); + static void deleteUnicodeStringKey(void *obj); UHashtable *map; }; @@ -928,6 +931,7 @@ StringToCEsMap::StringToCEsMap() &status); uhash_setValueDeleter(map, deleteCEList); + uhash_setKeyDeleter(map, deleteUnicodeStringKey); } StringToCEsMap::~StringToCEsMap() @@ -954,6 +958,13 @@ void StringToCEsMap::deleteCEList(void *obj) delete list; } +void StringToCEsMap::deleteUnicodeStringKey(void *obj) +{ + UnicodeString *key = (UnicodeString *) obj; + + delete key; +} + static void buildData(UCollator *coll, USet *charsToTest, StringToCEsMap *charsToCEList, CEToStringsMap *ceToCharsStartingWith) { int32_t itemCount = uset_getItemCount(charsToTest); @@ -1669,7 +1680,8 @@ void SSearchTest::monkeyTest(char *params) uset_close(contractions); uset_close(expansions); uset_close(charsToTest); - + uset_close(letters); + ucol_close(coll); } diff --git a/icu4c/source/test/intltest/tzrulets.cpp b/icu4c/source/test/intltest/tzrulets.cpp index 1e3d52e044e..f435e0ebb9f 100644 --- a/icu4c/source/test/intltest/tzrulets.cpp +++ b/icu4c/source/test/intltest/tzrulets.cpp @@ -2065,6 +2065,7 @@ TimeZoneRuleTest::TestT6216(void) { errln((UnicodeString)"FAIL: Invalid offset at time(" + times[j] + "):" + offset + " Expected:" + Expected[i][j]); } } + delete vtz; } }