]> granicus.if.org Git - icu/commitdiff
ICU-21763 UVector cleanup, continued.
authorAndy Heninger <andy.heninger@gmail.com>
Fri, 12 Nov 2021 00:12:44 +0000 (16:12 -0800)
committerAndy Heninger <andy.heninger@gmail.com>
Tue, 30 Nov 2021 17:12:16 +0000 (09:12 -0800)
Revise uses of UVector in the next batch of files to better handle memory
allocation failures.  This is one of an ongoing series of commits to address
similar problems with UVector usage throughout ICU.

The changes primarily involve switching uses of UVector::addElementX() to the
new adoptElement() or addElement() functions, as appropriate, and using
LocalPointers for tracking memory ownership.

icu4c/source/common/normalizer2impl.cpp
icu4c/source/common/serv.cpp
icu4c/source/common/servls.cpp
icu4c/source/common/servnotf.cpp
icu4c/source/i18n/alphaindex.cpp
icu4c/source/i18n/collationdatabuilder.cpp
icu4c/source/i18n/rbt_set.cpp
icu4c/source/i18n/uspoof_conf.cpp
icu4c/source/i18n/uspoof_conf.h
icu4c/source/test/intltest/icusvtst.cpp

index 5bfd49e8cb9e2ace5a9255e554a463f7fe1fbc3a..e6bd75e71738890506922135db6b6ff935c802cb 100644 (file)
@@ -2496,15 +2496,18 @@ void CanonIterData::addToStartSet(UChar32 origin, UChar32 decompLead, UErrorCode
         // origin is not the first character, or it is U+0000.
         UnicodeSet *set;
         if((canonValue&CANON_HAS_SET)==0) {
-            set=new UnicodeSet;
-            if(set==NULL) {
-                errorCode=U_MEMORY_ALLOCATION_ERROR;
+            LocalPointer<UnicodeSet> lpSet(new UnicodeSet, errorCode);
+            set=lpSet.getAlias();
+            if(U_FAILURE(errorCode)) {
                 return;
             }
             UChar32 firstOrigin=(UChar32)(canonValue&CANON_VALUE_MASK);
             canonValue=(canonValue&~CANON_VALUE_MASK)|CANON_HAS_SET|(uint32_t)canonStartSets.size();
             umutablecptrie_set(mutableTrie, decompLead, canonValue, &errorCode);
-            canonStartSets.addElementX(set, errorCode);
+            canonStartSets.adoptElement(lpSet.orphan(), errorCode);
+            if (U_FAILURE(errorCode)) {
+                return;
+            }
             if(firstOrigin!=0) {
                 set->add(firstOrigin);
             }
index 0c54a4dce99225761bf71252a5250b690cde4632..c26dbca1a9c244a158111c7401a5f3307815887f 100644 (file)
@@ -625,10 +625,7 @@ ICUService::getVisibleIDs(UVector& result, const UnicodeString* matchID, UErrorC
                     }
                 }
 
-                LocalPointer<UnicodeString> idClone(new UnicodeString(*id), status);
-                if (U_SUCCESS(status) && idClone->isBogus()) {
-                    status = U_MEMORY_ALLOCATION_ERROR;
-                }
+                LocalPointer<UnicodeString> idClone(id->clone(), status);
                 result.adoptElement(idClone.orphan(), status);
             }
             delete fallbackKey;
index 7108afd4a5282b4aee70a765cd30944e389b8a74..98f0a8a12b0006a5b8a3495a0ded9e19638cb557 100644 (file)
@@ -179,7 +179,8 @@ private:
 
             length = other._ids.size();
             for(i = 0; i < length; ++i) {
-                _ids.addElementX(((UnicodeString *)other._ids.elementAt(i))->clone(), status);
+                LocalPointer<UnicodeString> clonedId(((UnicodeString *)other._ids.elementAt(i))->clone(), status);
+                _ids.adoptElement(clonedId.orphan(), status);
             }
 
             if(U_SUCCESS(status)) {
index 342e0d9f24d2a75fe493f98b5bbf6e940fdd78f9..10183d3a2dc1b2c8bf382e3d07ec0b320fdeed37 100644 (file)
@@ -49,7 +49,11 @@ ICUNotifier::addListener(const EventListener* l, UErrorCode& status)
         if (acceptsListener(*l)) {
             Mutex lmx(&notifyLock);
             if (listeners == NULL) {
-                listeners = new UVector(5, status);
+                LocalPointer<UVector> lpListeners(new UVector(5, status), status);
+                if (U_FAILURE(status)) {
+                    return;
+                }
+                listeners = lpListeners.orphan();
             } else {
                 for (int i = 0, e = listeners->size(); i < e; ++i) {
                     const EventListener* el = (const EventListener*)(listeners->elementAt(i));
@@ -59,7 +63,7 @@ ICUNotifier::addListener(const EventListener* l, UErrorCode& status)
                 }
             }
 
-            listeners->addElementX((void*)l, status); // cast away const
+            listeners->addElement((void*)l, status); // cast away const
         }
 #ifdef NOTIFIER_DEBUG
         else {
index 34407f677a6b267974691cb3e20270e45de3c570..88d63675a7c2cbaca3cb96e1b5acec982278fd14 100644 (file)
@@ -451,12 +451,11 @@ BucketList *AlphabeticIndex::createBucketList(UErrorCode &errorCode) const {
     bucketList->setDeleter(uprv_deleteUObject);
 
     // underflow bucket
-    Bucket *bucket = new Bucket(getUnderflowLabel(), emptyString_, U_ALPHAINDEX_UNDERFLOW);
-    if (bucket == NULL) {
-        errorCode = U_MEMORY_ALLOCATION_ERROR;
+    LocalPointer<Bucket> bucket(new Bucket(getUnderflowLabel(), emptyString_, U_ALPHAINDEX_UNDERFLOW), errorCode);
+    if (U_FAILURE(errorCode)) {
         return NULL;
     }
-    bucketList->addElementX(bucket, errorCode);
+    bucketList->adoptElement(bucket.orphan(), errorCode);
     if (U_FAILURE(errorCode)) { return NULL; }
 
     UnicodeString temp;
@@ -481,28 +480,24 @@ BucketList *AlphabeticIndex::createBucketList(UErrorCode &errorCode) const {
             if (skippedScript && bucketList->size() > 1) {
                 // We are skipping one or more scripts,
                 // and we are not just getting out of the underflow label.
-                bucket = new Bucket(getInflowLabel(), inflowBoundary, U_ALPHAINDEX_INFLOW);
-                if (bucket == NULL) {
-                    errorCode = U_MEMORY_ALLOCATION_ERROR;
-                    return NULL;
-                }
-                bucketList->addElementX(bucket, errorCode);
+                bucket.adoptInsteadAndCheckErrorCode(
+                    new Bucket(getInflowLabel(), inflowBoundary, U_ALPHAINDEX_INFLOW), errorCode);
+                bucketList->adoptElement(bucket.orphan(), errorCode);
+                if (U_FAILURE(errorCode)) { return nullptr; }
             }
         }
         // Add a bucket with the current label.
-        bucket = new Bucket(fixLabel(current, temp), current, U_ALPHAINDEX_NORMAL);
-        if (bucket == NULL) {
-            errorCode = U_MEMORY_ALLOCATION_ERROR;
-            return NULL;
-        }
-        bucketList->addElementX(bucket, errorCode);
+        bucket.adoptInsteadAndCheckErrorCode(
+            new Bucket(fixLabel(current, temp), current, U_ALPHAINDEX_NORMAL), errorCode);
+        bucketList->adoptElement(bucket.orphan(), errorCode);
+        if (U_FAILURE(errorCode)) { return nullptr; }
         // Remember ASCII and Pinyin buckets for Pinyin redirects.
         UChar c;
         if (current.length() == 1 && 0x41 <= (c = current.charAt(0)) && c <= 0x5A) {  // A-Z
-            asciiBuckets[c - 0x41] = bucket;
+            asciiBuckets[c - 0x41] = (Bucket *)bucketList->lastElement();
         } else if (current.length() == BASE_LENGTH + 1 && current.startsWith(BASE, BASE_LENGTH) &&
                 0x41 <= (c = current.charAt(BASE_LENGTH)) && c <= 0x5A) {
-            pinyinBuckets[c - 0x41] = bucket;
+            pinyinBuckets[c - 0x41] = (Bucket *)bucketList->lastElement();
             hasPinyin = TRUE;
         }
         // Check for multiple primary weights.
@@ -526,15 +521,16 @@ BucketList *AlphabeticIndex::createBucketList(UErrorCode &errorCode) const {
                     // to the previous single-character bucket.
                     // For example, after ... Q R S Sch we add Sch\uFFFF->S
                     // and after ... Q R S Sch Sch\uFFFF St we add St\uFFFF->S.
-                    bucket = new Bucket(emptyString_,
+                    bucket.adoptInsteadAndCheckErrorCode(new Bucket(emptyString_,
                         UnicodeString(current).append((UChar)0xFFFF),
-                        U_ALPHAINDEX_NORMAL);
-                    if (bucket == NULL) {
-                        errorCode = U_MEMORY_ALLOCATION_ERROR;
+                        U_ALPHAINDEX_NORMAL),
+                        errorCode);
+                    if (U_FAILURE(errorCode)) {
                         return NULL;
                     }
                     bucket->displayBucket_ = singleBucket;
-                    bucketList->addElementX(bucket, errorCode);
+                    bucketList->adoptElement(bucket.orphan(), errorCode);
+                    if (U_FAILURE(errorCode)) { return nullptr; }
                     hasInvisibleBuckets = TRUE;
                     break;
                 }
@@ -553,12 +549,10 @@ BucketList *AlphabeticIndex::createBucketList(UErrorCode &errorCode) const {
         return bl;
     }
     // overflow bucket
-    bucket = new Bucket(getOverflowLabel(), *scriptUpperBoundary, U_ALPHAINDEX_OVERFLOW);
-    if (bucket == NULL) {
-        errorCode = U_MEMORY_ALLOCATION_ERROR;
-        return NULL;
-    }
-    bucketList->addElementX(bucket, errorCode); // final
+    bucket.adoptInsteadAndCheckErrorCode(
+        new Bucket(getOverflowLabel(), *scriptUpperBoundary, U_ALPHAINDEX_OVERFLOW), errorCode);
+    bucketList->adoptElement(bucket.orphan(), errorCode); // final
+    if (U_FAILURE(errorCode)) { return nullptr; }
 
     if (hasPinyin) {
         // Redirect Pinyin buckets.
@@ -589,7 +583,7 @@ BucketList *AlphabeticIndex::createBucketList(UErrorCode &errorCode) const {
     int32_t i = bucketList->size() - 1;
     Bucket *nextBucket = getBucket(*bucketList, i);
     while (--i > 0) {
-        bucket = getBucket(*bucketList, i);
+        Bucket *bucket = getBucket(*bucketList, i);
         if (bucket->displayBucket_ != NULL) {
             continue;  // skip invisible buckets
         }
@@ -609,9 +603,9 @@ BucketList *AlphabeticIndex::createBucketList(UErrorCode &errorCode) const {
     // Do not call publicBucketList->setDeleter():
     // This vector shares its objects with the bucketList.
     for (int32_t j = 0; j < bucketList->size(); ++j) {
-        bucket = getBucket(*bucketList, j);
+        Bucket *bucket = getBucket(*bucketList, j);
         if (bucket->displayBucket_ == NULL) {
-            publicBucketList->addElementX(bucket, errorCode);
+            publicBucketList->addElement(bucket, errorCode);
         }
     }
     if (U_FAILURE(errorCode)) { return NULL; }
@@ -679,13 +673,13 @@ void AlphabeticIndex::initBuckets(UErrorCode &errorCode) {
             bucket = bucket->displayBucket_;
         }
         if (bucket->records_ == NULL) {
-            bucket->records_ = new UVector(errorCode);
-            if (bucket->records_ == NULL) {
-                errorCode = U_MEMORY_ALLOCATION_ERROR;
+            LocalPointer<UVector> records(new UVector(errorCode), errorCode);
+            if (U_FAILURE(errorCode)) {
                 return;
             }
+            bucket->records_ = records.orphan();
         }
-        bucket->records_->addElementX(r, errorCode);
+        bucket->records_->addElement(r, errorCode);
     }
 }
 
@@ -1011,12 +1005,11 @@ UVector *AlphabeticIndex::firstStringsInScript(UErrorCode &status) {
             // and the one for unassigned implicit weights (Cn).
             continue;
         }
-        UnicodeString *s = new UnicodeString(boundary);
-        if (s == NULL) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-            return NULL;
+        LocalPointer<UnicodeString> s(new UnicodeString(boundary), status);
+        dest->adoptElement(s.orphan(), status);
+        if (U_FAILURE(status)) {
+            return nullptr;
         }
-        dest->addElementX(s, status);
     }
     return dest.orphan();
 }
@@ -1067,19 +1060,18 @@ AlphabeticIndex & AlphabeticIndex::addRecord(const UnicodeString &name, const vo
         return *this;
     }
     if (inputList_ == NULL) {
-        inputList_ = new UVector(status);
-        if (inputList_ == NULL) {
-            status = U_MEMORY_ALLOCATION_ERROR;
+        LocalPointer<UVector> inputList(new UVector(status), status);
+        if (U_FAILURE(status)) {
             return *this;
         }
+        inputList_ = inputList.orphan();
         inputList_->setDeleter(alphaIndex_deleteRecord);
     }
-    Record *r = new Record(name, data);
-    if (r == NULL) {
-        status = U_MEMORY_ALLOCATION_ERROR;
+    LocalPointer<Record> r(new Record(name, data), status);
+    inputList_->adoptElement(r.orphan(), status);
+    if (U_FAILURE(status)) {
         return *this;
     }
-    inputList_->addElementX(r, status);
     clearBuckets();
     //std::string ss;
     //std::string ss2;
index 25050aa777e681e763c589f11dfa3d04e5a99ccf..b10de993c279a32b2fc0dcbab314cc7affae1519 100644 (file)
@@ -522,12 +522,11 @@ CollationDataBuilder::addConditionalCE32(const UnicodeString &context, uint32_t
         errorCode = U_BUFFER_OVERFLOW_ERROR;
         return -1;
     }
-    ConditionalCE32 *cond = new ConditionalCE32(context, ce32);
-    if(cond == NULL) {
-        errorCode = U_MEMORY_ALLOCATION_ERROR;
+    LocalPointer<ConditionalCE32> cond(new ConditionalCE32(context, ce32), errorCode);
+    conditionalCE32s.adoptElement(cond.orphan(), errorCode);
+    if(U_FAILURE(errorCode)) {
         return -1;
     }
-    conditionalCE32s.addElementX(cond, errorCode);
     return index;
 }
 
index abc4413c2c6f61aa4f5875d40d47f4c148280760..6835c03a698b96ae730e22d9323226c23fe3eb58 100644 (file)
@@ -163,16 +163,13 @@ U_NAMESPACE_BEGIN
 /**
  * Construct a new empty rule set.
  */
-TransliterationRuleSet::TransliterationRuleSet(UErrorCode& status) : UMemory() {
-    ruleVector = new UVector(&_deleteRule, NULL, status);
+TransliterationRuleSet::TransliterationRuleSet(UErrorCode& status) :
+        UMemory(), ruleVector(nullptr), rules(nullptr), index {}, maxContextLength(0) {
+    LocalPointer<UVector> lpRuleVector(new UVector(_deleteRule, nullptr, status), status);
     if (U_FAILURE(status)) {
         return;
     }
-    if (ruleVector == NULL) {
-        status = U_MEMORY_ALLOCATION_ERROR;
-    }
-    rules = NULL;
-    maxContextLength = 0;
+    ruleVector = lpRuleVector.orphan();
 }
 
 /**
@@ -180,27 +177,24 @@ TransliterationRuleSet::TransliterationRuleSet(UErrorCode& status) : UMemory() {
  */
 TransliterationRuleSet::TransliterationRuleSet(const TransliterationRuleSet& other) :
     UMemory(other),
-    ruleVector(0),
-    rules(0),
+    ruleVector(nullptr),
+    rules(nullptr),
     maxContextLength(other.maxContextLength) {
 
     int32_t i, len;
     uprv_memcpy(index, other.index, sizeof(index));
     UErrorCode status = U_ZERO_ERROR;
-    ruleVector = new UVector(&_deleteRule, NULL, status);
-    if (other.ruleVector != 0 && ruleVector != 0 && U_SUCCESS(status)) {
+    LocalPointer<UVector> lpRuleVector(new UVector(_deleteRule, nullptr, status), status);
+    if (U_FAILURE(status)) {
+        return;
+    }
+    ruleVector = lpRuleVector.orphan();
+    if (other.ruleVector != nullptr && U_SUCCESS(status)) {
         len = other.ruleVector->size();
         for (i=0; i<len && U_SUCCESS(status); ++i) {
-            TransliterationRule *tempTranslitRule = new TransliterationRule(*(TransliterationRule*)other.ruleVector->elementAt(i));
-            // Null pointer test
-            if (tempTranslitRule == NULL) {
-                status = U_MEMORY_ALLOCATION_ERROR;
-                break;
-            }
-            ruleVector->addElementX(tempTranslitRule, status);
-            if (U_FAILURE(status)) {
-                break;
-            }
+            LocalPointer<TransliterationRule> tempTranslitRule(
+                new TransliterationRule(*(TransliterationRule*)other.ruleVector->elementAt(i)), status);
+            ruleVector->adoptElement(tempTranslitRule.orphan(), status);
         }
     }
     if (other.rules != 0 && U_SUCCESS(status)) {
@@ -247,11 +241,11 @@ int32_t TransliterationRuleSet::getMaximumContextLength(void) const {
  */
 void TransliterationRuleSet::addRule(TransliterationRule* adoptedRule,
                                      UErrorCode& status) {
+    LocalPointer<TransliterationRule> lpAdoptedRule(adoptedRule);
+    ruleVector->adoptElement(lpAdoptedRule.orphan(), status);
     if (U_FAILURE(status)) {
-        delete adoptedRule;
         return;
     }
-    ruleVector->addElementX(adoptedRule, status);
 
     int32_t len;
     if ((len = adoptedRule->getContextLength()) > maxContextLength) {
@@ -316,7 +310,7 @@ void TransliterationRuleSet::freeze(UParseError& parseError,UErrorCode& status)
         for (j=0; j<n; ++j) {
             if (indexValue[j] >= 0) {
                 if (indexValue[j] == x) {
-                    v.addElementX(ruleVector->elementAt(j), status);
+                    v.addElement(ruleVector->elementAt(j), status);
                 }
             } else {
                 // If the indexValue is < 0, then the first key character is
@@ -325,13 +319,16 @@ void TransliterationRuleSet::freeze(UParseError& parseError,UErrorCode& status)
                 // rarely, so we seldom treat this code path.
                 TransliterationRule* r = (TransliterationRule*) ruleVector->elementAt(j);
                 if (r->matchesIndexValue((uint8_t)x)) {
-                    v.addElementX(r, status);
+                    v.addElement(r, status);
                 }
             }
         }
     }
     uprv_free(indexValue);
     index[256] = v.size();
+    if (U_FAILURE(status)) {
+        return;
+    }
 
     /* Freeze things into an array.
      */
index 04081cabfb0738ebe38b21712196b59acfd2662a..172c0711afbad3f763654e19c1fe875e2c27699d 100644 (file)
@@ -63,23 +63,24 @@ U_NAMESPACE_USE
 //         at the same time
 //
 
-SPUString::SPUString(UnicodeString *s) {
-    fStr = s;
+SPUString::SPUString(LocalPointer<UnicodeString> s) {
+    fStr = std::move(s);
     fCharOrStrTableIndex = 0;
 }
 
 
 SPUString::~SPUString() {
-    delete fStr;
 }
 
 
-SPUStringPool::SPUStringPool(UErrorCode &status) : fVec(NULL), fHash(NULL) {
-    fVec = new UVector(status);
-    if (fVec == NULL) {
-        status = U_MEMORY_ALLOCATION_ERROR;
+SPUStringPool::SPUStringPool(UErrorCode &status) : fVec(nullptr), fHash(nullptr) {
+    LocalPointer<UVector> vec(new UVector(status), status);
+    if (U_FAILURE(status)) {
         return;
     }
+    vec->setDeleter(
+        [](void *obj) {delete (SPUString *)obj;});
+    fVec = vec.orphan();
     fHash = uhash_open(uhash_hashUnicodeString,           // key hash function
                        uhash_compareUnicodeString,        // Key Comparator
                        NULL,                              // Value Comparator
@@ -88,11 +89,6 @@ SPUStringPool::SPUStringPool(UErrorCode &status) : fVec(NULL), fHash(NULL) {
 
 
 SPUStringPool::~SPUStringPool() {
-    int i;
-    for (i=fVec->size()-1; i>=0; i--) {
-        SPUString *s = static_cast<SPUString *>(fVec->elementAt(i));
-        delete s;
-    }
     delete fVec;
     uhash_close(fHash);
 }
@@ -135,18 +131,21 @@ void SPUStringPool::sort(UErrorCode &status) {
 
 
 SPUString *SPUStringPool::addString(UnicodeString *src, UErrorCode &status) {
+    LocalPointer<UnicodeString> lpSrc(src);
+    if (U_FAILURE(status)) {
+        return nullptr;
+    }
     SPUString *hashedString = static_cast<SPUString *>(uhash_get(fHash, src));
-    if (hashedString != NULL) {
-        delete src;
-    } else {
-        hashedString = new SPUString(src);
-        if (hashedString == NULL) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-            return NULL;
-        }
-        uhash_put(fHash, src, hashedString, &status);
-        fVec->addElementX(hashedString, status);
+    if (hashedString != nullptr) {
+        return hashedString;
+    }
+    LocalPointer<SPUString> spuStr(new SPUString(std::move(lpSrc)), status);
+    hashedString = spuStr.getAlias();
+    fVec->adoptElement(spuStr.orphan(), status);
+    if (U_FAILURE(status)) {
+        return nullptr;
     }
+    uhash_put(fHash, src, hashedString, &status);
     return hashedString;
 }
 
index 600d7ea42a430d8e4d0308b92504cd210db23141..1eeecdfd5e40d4105c221195446ad320fef12bf2 100644 (file)
@@ -39,11 +39,12 @@ U_NAMESPACE_BEGIN
 //              Instances of SPUString exist during the compilation process only.
 
 struct SPUString : public UMemory {
-    UnicodeString  *fStr;             // The actual string.
-    int32_t         fCharOrStrTableIndex;   // Index into the final runtime data for this
-                                      // string (or, for length 1, the single string char
-                                      // itself, there being no string table entry for it.)
-    SPUString(UnicodeString *s);
+    LocalPointer<UnicodeString> fStr;     // The actual string.
+    int32_t      fCharOrStrTableIndex;    // Index into the final runtime data for this
+                                          // string (or, for length 1, the single string char
+                                          // itself, there being no string table entry for it.)
+
+    SPUString(LocalPointer<UnicodeString> s);
     ~SPUString();
 };
 
index aa2d9ca05ce73e43895b7d9b7e619ba8b84312a0..cb863c5b71b770044698e2442d156d1c5aa43dd3 100644 (file)
@@ -558,7 +558,7 @@ class TestMultipleKeyStringFactory : public ICUServiceFactory {
         , _factoryID(factoryID + ": ") 
     {
         for (int i = 0; i < count; ++i) {
-            _ids.addElementX(new UnicodeString(ids[i]), _status);
+            _ids.adoptElement(new UnicodeString(ids[i]), _status);
         }
     }