ICU-21763 UVector cleanup in time zone code

Revise uses of UVector in time zone related code 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 involve switching uses of UVector::addElementX() to the new
adoptElement() or addElement() functions, as appropriate, and using
LocalPointers for tracking memory ownership.

diff --git a/icu4c/source/common/ucurr.cpp b/icu4c/source/common/ucurr.cpp
index 67aab4e8ffec2f004c9083945928aa93b52f65da..6e489e0563d41675639a0627a4487fb50d397cc1 100644 (file)
--- a/icu4c/source/common/ucurr.cpp
+++ b/icu4c/source/common/ucurr.cpp
@@ -254,7 +254,7 @@ currSymbolsEquiv_cleanup(void)
 }
 
 /**
- * Deleter for OlsonToMetaMappingEntry
+ * Deleter for IsoCodeEntry
  */
 static void U_CALLCONV
 deleteIsoCodeEntry(void *obj) {

diff --git a/icu4c/source/i18n/tzfmt.cpp b/icu4c/source/i18n/tzfmt.cpp
index ef3cfad80ce1d655a85911f78d558780357bb0bd..9d046c30c8f07bef560a02914e034d855e9615ae 100644 (file)
--- a/icu4c/source/i18n/tzfmt.cpp
+++ b/icu4c/source/i18n/tzfmt.cpp
@@ -2459,7 +2459,7 @@ TimeZoneFormat::parseOffsetPattern(const UnicodeString& pattern, OffsetFields re
                 if (itemType != GMTOffsetField::TEXT) {
                     if (GMTOffsetField::isValid(itemType, itemLength)) {
                         GMTOffsetField* fld = GMTOffsetField::createTimeField(itemType, static_cast<uint8_t>(itemLength), status);
-                        result->addElementX(fld, status);
+                        result->adoptElement(fld, status);
                         if (U_FAILURE(status)) {
                             break;
                         }
@@ -2485,7 +2485,7 @@ TimeZoneFormat::parseOffsetPattern(const UnicodeString& pattern, OffsetFields re
                         if (itemType == GMTOffsetField::TEXT) {
                             if (text.length() > 0) {
                                 GMTOffsetField* textfld = GMTOffsetField::createText(text, status);
-                                result->addElementX(textfld, status);
+                                result->adoptElement(textfld, status);
                                 if (U_FAILURE(status)) {
                                     break;
                                 }
@@ -2494,7 +2494,7 @@ TimeZoneFormat::parseOffsetPattern(const UnicodeString& pattern, OffsetFields re
                         } else {
                             if (GMTOffsetField::isValid(itemType, itemLength)) {
                                 GMTOffsetField* fld = GMTOffsetField::createTimeField(itemType, static_cast<uint8_t>(itemLength), status);
-                                result->addElementX(fld, status);
+                                result->adoptElement(fld, status);
                                 if (U_FAILURE(status)) {
                                     break;
                                 }
@@ -2512,7 +2512,7 @@ TimeZoneFormat::parseOffsetPattern(const UnicodeString& pattern, OffsetFields re
                     if (itemType != GMTOffsetField::TEXT) {
                         if (GMTOffsetField::isValid(itemType, itemLength)) {
                             GMTOffsetField* fld = GMTOffsetField::createTimeField(itemType, static_cast<uint8_t>(itemLength), status);
-                            result->addElementX(fld, status);
+                            result->adoptElement(fld, status);
                             if (U_FAILURE(status)) {
                                 break;
                             }
@@ -2532,12 +2532,12 @@ TimeZoneFormat::parseOffsetPattern(const UnicodeString& pattern, OffsetFields re
         if (itemType == GMTOffsetField::TEXT) {
             if (text.length() > 0) {
                 GMTOffsetField* tfld = GMTOffsetField::createText(text, status);
-                result->addElementX(tfld, status);
+                result->adoptElement(tfld, status);
             }
         } else {
             if (GMTOffsetField::isValid(itemType, itemLength)) {
                 GMTOffsetField* fld = GMTOffsetField::createTimeField(itemType, static_cast<uint8_t>(itemLength), status);
-                result->addElementX(fld, status);
+                result->adoptElement(fld, status);
             } else {
                 status = U_ILLEGAL_ARGUMENT_ERROR;
             }

diff --git a/icu4c/source/i18n/tzgnames.cpp b/icu4c/source/i18n/tzgnames.cpp
index ed5f42d7bc1d6d42820e3095328a319aa5d10644..d5ee45ced78db4d1bd68cbd06e6ca3fe198c5576 100644 (file)
--- a/icu4c/source/i18n/tzgnames.cpp
+++ b/icu4c/source/i18n/tzgnames.cpp
@@ -229,30 +229,27 @@ GNameSearchHandler::handleMatch(int32_t matchLength, const CharacterNode *node,
             if ((nameinfo->type & fTypes) != 0) {
                 // matches a requested type
                 if (fResults == NULL) {
-                    fResults = new UVector(uprv_free, NULL, status);
-                    if (fResults == NULL) {
-                        status = U_MEMORY_ALLOCATION_ERROR;
+                    LocalPointer<UVector> lpResults(new UVector(uprv_free, NULL, status), status);
+                    if (U_FAILURE(status)) {
+                        return false;
                     }
+                    fResults = lpResults.orphan();
                 }
-                if (U_SUCCESS(status)) {
-                    U_ASSERT(fResults != NULL);
-                    GMatchInfo *gmatch = (GMatchInfo *)uprv_malloc(sizeof(GMatchInfo));
-                    if (gmatch == NULL) {
-                        status = U_MEMORY_ALLOCATION_ERROR;
-                    } else {
-                        // add the match to the vector
-                        gmatch->gnameInfo = nameinfo;
-                        gmatch->matchLength = matchLength;
-                        gmatch->timeType = UTZFMT_TIME_TYPE_UNKNOWN;
-                        fResults->addElementX(gmatch, status);
-                        if (U_FAILURE(status)) {
-                            uprv_free(gmatch);
-                        } else {
-                            if (matchLength > fMaxMatchLen) {
-                                fMaxMatchLen = matchLength;
-                            }
-                        }
-                    }
+                GMatchInfo *gmatch = (GMatchInfo *)uprv_malloc(sizeof(GMatchInfo));
+                if (gmatch == NULL) {
+                    status = U_MEMORY_ALLOCATION_ERROR;
+                    return false;
+                }
+                // add the match to the vector
+                gmatch->gnameInfo = nameinfo;
+                gmatch->matchLength = matchLength;
+                gmatch->timeType = UTZFMT_TIME_TYPE_UNKNOWN;
+                fResults->adoptElement(gmatch, status);
+                if (U_FAILURE(status)) {
+                    return false;
+                }
+                if (matchLength > fMaxMatchLen) {
+                    fMaxMatchLen = matchLength;
                 }
             }
         }

diff --git a/icu4c/source/i18n/tznames.cpp b/icu4c/source/i18n/tznames.cpp
index 5c504d01cb63424e7ae6567ac34012b1a160e003..781f1cc161f9c544bdd514744b86b114c42d29fe 100644 (file)
--- a/icu4c/source/i18n/tznames.cpp
+++ b/icu4c/source/i18n/tznames.cpp
@@ -414,15 +414,12 @@ TimeZoneNames::MatchInfoCollection::addZone(UTimeZoneNameType nameType, int32_t
     if (U_FAILURE(status)) {
         return;
     }
-    MatchInfo* matchInfo = new MatchInfo(nameType, matchLength, &tzID, NULL);
-    if (matchInfo == NULL) {
-        status = U_MEMORY_ALLOCATION_ERROR;
-        return;
-    }
-    matches(status)->addElementX(matchInfo, status);
+    LocalPointer <MatchInfo> matchInfo(new MatchInfo(nameType, matchLength, &tzID, NULL), status);
+    UVector *matchesVec = matches(status);
     if (U_FAILURE(status)) {
-        delete matchInfo;
+        return;
     }
+    matchesVec->adoptElement(matchInfo.orphan(), status);
 }
 
 void
@@ -431,15 +428,12 @@ TimeZoneNames::MatchInfoCollection::addMetaZone(UTimeZoneNameType nameType, int3
     if (U_FAILURE(status)) {
         return;
     }
-    MatchInfo* matchInfo = new MatchInfo(nameType, matchLength, NULL, &mzID);
-    if (matchInfo == NULL) {
-        status = U_MEMORY_ALLOCATION_ERROR;
-        return;
-    }
-    matches(status)->addElementX(matchInfo, status);
+    LocalPointer<MatchInfo> matchInfo(new MatchInfo(nameType, matchLength, NULL, &mzID), status);
+    UVector *matchesVec = matches(status);
     if (U_FAILURE(status)) {
-        delete matchInfo;
+        return;
     }
+    matchesVec->adoptElement(matchInfo.orphan(), status);
 }
 
 int32_t

diff --git a/icu4c/source/i18n/tznames_impl.cpp b/icu4c/source/i18n/tznames_impl.cpp
index d450b7456489bf750aa0b0de8ad3b8c54fba83be..69991dfef4b5c022a3b3ecada673aba5c13cdff9 100644 (file)
--- a/icu4c/source/i18n/tznames_impl.cpp
+++ b/icu4c/source/i18n/tznames_impl.cpp
@@ -148,19 +148,29 @@ CharacterNode::addValue(void *value, UObjectDeleter *valueDeleter, UErrorCode &s
         if (!fHasValuesVector) {
             // There is only one value so far, and not in a vector yet.
             // Create a vector and add the old value.
-            UVector *values = new UVector(valueDeleter, NULL, DEFAULT_CHARACTERNODE_CAPACITY, status);
+            LocalPointer<UVector> values(
+                new UVector(valueDeleter, NULL, DEFAULT_CHARACTERNODE_CAPACITY, status), status);
             if (U_FAILURE(status)) {
                 if (valueDeleter) {
                     valueDeleter(value);
                 }
                 return;
             }
-            values->addElementX(fValues, status);
-            fValues = values;
+            if (values->hasDeleter()) {
+                values->adoptElement(fValues, status);
+            } else {
+                values->addElement(fValues, status);
+            }
+            fValues = values.orphan();
             fHasValuesVector = TRUE;
         }
         // Add the new value.
-        ((UVector *)fValues)->addElementX(value, status);
+        UVector *values = (UVector *)fValues;
+        if (values->hasDeleter()) {
+            values->adoptElement(value, status);
+        } else {
+            values->addElement(value, status);
+        }
     }
 }
 
@@ -219,10 +229,8 @@ void
 TextTrieMap::put(const UChar *key, void *value, UErrorCode &status) {
     fIsEmpty = FALSE;
     if (fLazyContents == NULL) {
-        fLazyContents = new UVector(status);
-        if (fLazyContents == NULL) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-        }
+        LocalPointer<UVector> lpLazyContents(new UVector(status), status);
+        fLazyContents = lpLazyContents.orphan();
     }
     if (U_FAILURE(status)) {
         if (fValueDeleter) {
@@ -233,7 +241,7 @@ TextTrieMap::put(const UChar *key, void *value, UErrorCode &status) {
     U_ASSERT(fLazyContents != NULL);
 
     UChar *s = const_cast<UChar *>(key);
-    fLazyContents->addElementX(s, status);
+    fLazyContents->addElement(s, status);
     if (U_FAILURE(status)) {
         if (fValueDeleter) {
             fValueDeleter((void*) key);
@@ -241,7 +249,7 @@ TextTrieMap::put(const UChar *key, void *value, UErrorCode &status) {
         return;
     }
 
-    fLazyContents->addElementX(value, status);
+    fLazyContents->addElement(value, status);
 }
 
 void
@@ -854,7 +862,7 @@ class MetaZoneIDsEnumeration : public StringEnumeration {
 public:
     MetaZoneIDsEnumeration();
     MetaZoneIDsEnumeration(const UVector& mzIDs);
-    MetaZoneIDsEnumeration(UVector* mzIDs);
+    MetaZoneIDsEnumeration(LocalPointer<UVector> mzIDs);
     virtual ~MetaZoneIDsEnumeration();
     static UClassID U_EXPORT2 getStaticClassID(void);
     virtual UClassID getDynamicClassID(void) const override;
@@ -865,7 +873,7 @@ private:
     int32_t fLen;
     int32_t fPos;
     const UVector* fMetaZoneIDs;
-    UVector *fLocalVector;
+    LocalPointer<UVector> fLocalVector;
 };
 
 UOBJECT_DEFINE_RTTI_IMPLEMENTATION(MetaZoneIDsEnumeration)
@@ -879,8 +887,9 @@ MetaZoneIDsEnumeration::MetaZoneIDsEnumeration(const UVector& mzIDs)
     fLen = fMetaZoneIDs->size();
 }
 
-MetaZoneIDsEnumeration::MetaZoneIDsEnumeration(UVector *mzIDs)
-: fLen(0), fPos(0), fMetaZoneIDs(mzIDs), fLocalVector(mzIDs) {
+MetaZoneIDsEnumeration::MetaZoneIDsEnumeration(LocalPointer<UVector> mzIDs)
+: fLen(0), fPos(0), fMetaZoneIDs(nullptr), fLocalVector(std::move(mzIDs)) {
+    fMetaZoneIDs = fLocalVector.getAlias();
     if (fMetaZoneIDs) {
         fLen = fMetaZoneIDs->size();
     }
@@ -906,9 +915,6 @@ MetaZoneIDsEnumeration::count(UErrorCode& /*status*/) const {
 }
 
 MetaZoneIDsEnumeration::~MetaZoneIDsEnumeration() {
-    if (fLocalVector) {
-        delete fLocalVector;
-    }
 }
 
 
@@ -1153,28 +1159,23 @@ TimeZoneNamesImpl::_getAvailableMetaZoneIDs(const UnicodeString& tzID, UErrorCod
         return new MetaZoneIDsEnumeration();
     }
 
-    MetaZoneIDsEnumeration *senum = NULL;
-    UVector* mzIDs = new UVector(NULL, uhash_compareUChars, status);
-    if (mzIDs == NULL) {
-        status = U_MEMORY_ALLOCATION_ERROR;
-    }
+    LocalPointer<MetaZoneIDsEnumeration> senum;
+    LocalPointer<UVector> mzIDs(new UVector(NULL, uhash_compareUChars, status), status);
     if (U_SUCCESS(status)) {
-        U_ASSERT(mzIDs != NULL);
+        U_ASSERT(mzIDs.isValid());
         for (int32_t i = 0; U_SUCCESS(status) && i < mappings->size(); i++) {
 
             OlsonToMetaMappingEntry *map = (OlsonToMetaMappingEntry *)mappings->elementAt(i);
             const UChar *mzID = map->mzid;
             if (!mzIDs->contains((void *)mzID)) {
-                mzIDs->addElementX((void *)mzID, status);
+                mzIDs->addElement((void *)mzID, status);
             }
         }
         if (U_SUCCESS(status)) {
-            senum = new MetaZoneIDsEnumeration(mzIDs);
-        } else {
-            delete mzIDs;
+            senum.adoptInsteadAndCheckErrorCode(new MetaZoneIDsEnumeration(std::move(mzIDs)), status);
         }
     }
-    return senum;
+    return U_SUCCESS(status) ? senum.orphan() : nullptr;
 }
 
 UnicodeString&

diff --git a/icu4c/source/i18n/zonemeta.cpp b/icu4c/source/i18n/zonemeta.cpp
index b8afa4760f1823c9cde7d73bf300397045e6b65a..e60215c9988e6dcf849f3d9fb8a7a3b0bf0cb92d 100644 (file)
--- a/icu4c/source/i18n/zonemeta.cpp
+++ b/icu4c/source/i18n/zonemeta.cpp
@@ -97,21 +97,13 @@ deleteUCharString(void *obj) {
     uprv_free(entry);
 }
 
-/**
- * Deleter for UVector
- */
-static void U_CALLCONV
-deleteUVector(void *obj) {
-   delete (icu::UVector*) obj;
-}
-
 /**
  * Deleter for OlsonToMetaMappingEntry
  */
 static void U_CALLCONV
 deleteOlsonToMetaMappingEntry(void *obj) {
     icu::OlsonToMetaMappingEntry *entry = (icu::OlsonToMetaMappingEntry*)obj;
-    uprv_free(entry);
+    delete entry;
 }
 
 U_CDECL_END
@@ -477,11 +469,11 @@ ZoneMeta::getCanonicalCountry(const UnicodeString &tzid, UnicodeString &country,
                 UErrorCode ec = U_ZERO_ERROR;
                 if (singleZone) {
                     if (!gSingleZoneCountries->contains((void*)region)) {
-                        gSingleZoneCountries->addElementX((void*)region, ec);
+                        gSingleZoneCountries->addElement((void*)region, ec);
                     }
                 } else {
                     if (!gMultiZonesCountries->contains((void*)region)) {
-                        gMultiZonesCountries->addElementX((void*)region, ec);
+                        gMultiZonesCountries->addElement((void*)region, ec);
                     }
                 }
             }
@@ -550,7 +542,7 @@ static void U_CALLCONV olsonToMetaInit(UErrorCode &status) {
         gOlsonToMeta = NULL;
     } else {
         uhash_setKeyDeleter(gOlsonToMeta, deleteUCharString);
-        uhash_setValueDeleter(gOlsonToMeta, deleteUVector);
+        uhash_setValueDeleter(gOlsonToMeta, uprv_deleteUObject);
     }
 }
 
@@ -625,7 +617,7 @@ ZoneMeta::getMetazoneMappings(const UnicodeString &tzid) {
 
 UVector*
 ZoneMeta::createMetazoneMappings(const UnicodeString &tzid) {
-    UVector *mzMappings = NULL;
+    LocalPointer <UVector> mzMappings;
     UErrorCode status = U_ZERO_ERROR;
 
     UnicodeString canonicalID;
@@ -677,41 +669,32 @@ ZoneMeta::createMetazoneMappings(const UnicodeString &tzid) {
                     continue;
                 }
 
-                OlsonToMetaMappingEntry *entry = (OlsonToMetaMappingEntry*)uprv_malloc(sizeof(OlsonToMetaMappingEntry));
-                if (entry == NULL) {
-                    status = U_MEMORY_ALLOCATION_ERROR;
+                LocalPointer<OlsonToMetaMappingEntry> entry(new OlsonToMetaMappingEntry, status);
+                if (U_FAILURE(status)) {
                     break;
                 }
                 entry->mzid = mz_name;
                 entry->from = from;
                 entry->to = to;
 
-                if (mzMappings == NULL) {
-                    mzMappings = new UVector(deleteOlsonToMetaMappingEntry, NULL, status);
+                if (mzMappings.isNull()) {
+                    mzMappings.adoptInsteadAndCheckErrorCode(
+                        new UVector(deleteOlsonToMetaMappingEntry, nullptr, status), status);
                     if (U_FAILURE(status)) {
-                        delete mzMappings;
-                        mzMappings = NULL;
-                        uprv_free(entry);
                         break;
                     }
                 }
 
-                mzMappings->addElementX(entry, status);
+                mzMappings->adoptElement(entry.orphan(), status);
                 if (U_FAILURE(status)) {
                     break;
                 }
             }
             ures_close(mz);
-            if (U_FAILURE(status)) {
-                if (mzMappings != NULL) {
-                    delete mzMappings;
-                    mzMappings = NULL;
-                }
-            }
         }
     }
     ures_close(rb);
-    return mzMappings;
+    return U_SUCCESS(status) ? mzMappings.orphan() : nullptr;
 }
 
 UnicodeString& U_EXPORT2
@@ -775,6 +758,7 @@ static void U_CALLCONV initAvailableMetaZoneIDs () {
     // No valueDeleter, because the vector maintain the value objects
     gMetaZoneIDs = new UVector(NULL, uhash_compareUChars, status);
     if (U_FAILURE(status) || gMetaZoneIDs == NULL) {
+        delete gMetaZoneIDs;
         gMetaZoneIDs = NULL;
         uhash_close(gMetaZoneIDTable);
         gMetaZoneIDTable = NULL;
@@ -792,20 +776,22 @@ static void U_CALLCONV initAvailableMetaZoneIDs () {
         }
         const char *mzID = ures_getKey(res.getAlias());
         int32_t len = static_cast<int32_t>(uprv_strlen(mzID));
-        UChar *uMzID = (UChar*)uprv_malloc(sizeof(UChar) * (len + 1));
-        if (uMzID == NULL) {
+        LocalMemory<UChar> uMzID((UChar*)uprv_malloc(sizeof(UChar) * (len + 1)));
+        if (uMzID.isNull()) {
             status = U_MEMORY_ALLOCATION_ERROR;
             break;
         }
-        u_charsToUChars(mzID, uMzID, len);
+        u_charsToUChars(mzID, uMzID.getAlias(), len);
         uMzID[len] = 0;
-        UnicodeString *usMzID = new UnicodeString(uMzID);
-        if (uhash_get(gMetaZoneIDTable, usMzID) == NULL) {
-            gMetaZoneIDs->addElementX((void *)uMzID, status);
-            uhash_put(gMetaZoneIDTable, (void *)usMzID, (void *)uMzID, &status);
-        } else {
-            uprv_free(uMzID);
-            delete usMzID;
+        LocalPointer<UnicodeString> usMzID(new UnicodeString(uMzID.getAlias()), status);
+        if (U_FAILURE(status)) {
+            break;
+        }
+        if (uhash_get(gMetaZoneIDTable, usMzID.getAlias()) == NULL) {
+            // Note: gMetaZoneIDTable adopts its keys, but not its values.
+            //       gMetaZoneIDs adopts its values.
+            uhash_put(gMetaZoneIDTable, usMzID.orphan(), uMzID.getAlias(), &status);
+            gMetaZoneIDs->adoptElement(uMzID.orphan(), status);
         }
     }
     ures_close(bundle);

diff --git a/icu4c/source/i18n/zonemeta.h b/icu4c/source/i18n/zonemeta.h
index f21399342b9e67789830745e399dc28a1ebf250a..dd4fec957fedaef152b0f9e93841ce9244d7462a 100644 (file)
--- a/icu4c/source/i18n/zonemeta.h
+++ b/icu4c/source/i18n/zonemeta.h
@@ -18,11 +18,11 @@
 
 U_NAMESPACE_BEGIN
 
-typedef struct OlsonToMetaMappingEntry {
+struct OlsonToMetaMappingEntry : public UMemory {
     const UChar *mzid; // const because it's a reference to a resource bundle string.
     UDate from;
     UDate to;
-} OlsonToMetaMappingEntry;
+};
 
 class UVector;
 class TimeZone;