ICU-21763 UVector cleanup in Formatting Code

Revise uses of UVector in Formatting 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 primarily 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/i18n/dtfmtsym.cpp b/icu4c/source/i18n/dtfmtsym.cpp
index ab5f2b612c1f0efdb3a4e0a3065e424c6eef44cb..134b919f06ea596fcedd449c36dbe130c7f21eb4 100644 (file)
--- a/icu4c/source/i18n/dtfmtsym.cpp
+++ b/icu4c/source/i18n/dtfmtsym.cpp
@@ -1574,26 +1574,20 @@ struct CalendarDataSink : public ResourceSink {
                                                        errorCode);
                     if (U_FAILURE(errorCode)) { return; }
                 }
-                LocalPointer<UnicodeString> aliasRelativePathCopy(new UnicodeString(aliasRelativePath), errorCode);
-                resourcesToVisitNext->addElementX(aliasRelativePathCopy.getAlias(), errorCode);
+                LocalPointer<UnicodeString> aliasRelativePathCopy(aliasRelativePath.clone(), errorCode);
+                resourcesToVisitNext->adoptElement(aliasRelativePathCopy.orphan(), errorCode);
                 if (U_FAILURE(errorCode)) { return; }
-                // Only release ownership after resourcesToVisitNext takes it (no error happened):
-                aliasRelativePathCopy.orphan();
                 continue;
 
             } else if (aliasType == SAME_CALENDAR) {
                 // Register same-calendar alias
                 if (arrays.get(aliasRelativePath) == NULL && maps.get(aliasRelativePath) == NULL) {
-                    LocalPointer<UnicodeString> aliasRelativePathCopy(new UnicodeString(aliasRelativePath), errorCode);
-                    aliasPathPairs.addElementX(aliasRelativePathCopy.getAlias(), errorCode);
+                    LocalPointer<UnicodeString> aliasRelativePathCopy(aliasRelativePath.clone(), errorCode);
+                    aliasPathPairs.adoptElement(aliasRelativePathCopy.orphan(), errorCode);
                     if (U_FAILURE(errorCode)) { return; }
-                    // Only release ownership after aliasPathPairs takes it (no error happened):
-                    aliasRelativePathCopy.orphan();
-                    LocalPointer<UnicodeString> keyUStringCopy(new UnicodeString(keyUString), errorCode);
-                    aliasPathPairs.addElementX(keyUStringCopy.getAlias(), errorCode);
+                    LocalPointer<UnicodeString> keyUStringCopy(keyUString.clone(), errorCode);
+                    aliasPathPairs.adoptElement(keyUStringCopy.orphan(), errorCode);
                     if (U_FAILURE(errorCode)) { return; }
-                    // Only release ownership after aliasPathPairs takes it (no error happened):
-                    keyUStringCopy.orphan();
                 }
                 continue;
             }
@@ -1760,16 +1754,12 @@ struct CalendarDataSink : public ResourceSink {
             if (U_FAILURE(errorCode)) { return; }
             if (aliasType == SAME_CALENDAR) {
                 // Store the alias path and the current path on aliasPathPairs
-                LocalPointer<UnicodeString> aliasRelativePathCopy(new UnicodeString(aliasRelativePath), errorCode);
-                aliasPathPairs.addElementX(aliasRelativePathCopy.getAlias(), errorCode);
+                LocalPointer<UnicodeString> aliasRelativePathCopy(aliasRelativePath.clone(), errorCode);
+                aliasPathPairs.adoptElement(aliasRelativePathCopy.orphan(), errorCode);
                 if (U_FAILURE(errorCode)) { return; }
-                // Only release ownership after aliasPathPairs takes it (no error happened):
-                aliasRelativePathCopy.orphan();
-                LocalPointer<UnicodeString> pathCopy(new UnicodeString(path), errorCode);
-                aliasPathPairs.addElementX(pathCopy.getAlias(), errorCode);
+                LocalPointer<UnicodeString> pathCopy(path.clone(), errorCode);
+                aliasPathPairs.adoptElement(pathCopy.orphan(), errorCode);
                 if (U_FAILURE(errorCode)) { return; }
-                // Only release ownership after aliasPathPairs takes it (no error happened):
-                pathCopy.orphan();
 
                 // Drop the latest key on the path and continue
                 path.retainBetween(0, pathLength);

diff --git a/icu4c/source/i18n/dtptngen.cpp b/icu4c/source/i18n/dtptngen.cpp
index 6aee1750f906171f88b3a17b74ac972b7ec14174..987d83663acd94622e9a33bbace12522f2978365 100644 (file)
--- a/icu4c/source/i18n/dtptngen.cpp
+++ b/icu4c/source/i18n/dtptngen.cpp
@@ -2788,16 +2788,17 @@ DTSkeletonEnumeration::DTSkeletonEnumeration(PatternMap& patternMap, dtStrEnum t
                     break;
             }
             if ( !isCanonicalItem(s) ) {
-                LocalPointer<UnicodeString> newElem(new UnicodeString(s), status);
+                LocalPointer<UnicodeString> newElem(s.clone(), status);
                 if (U_FAILURE(status)) { 
                     return;
                 }
-                fSkeletons->addElementX(newElem.getAlias(), status);
+                fSkeletons->addElement(newElem.getAlias(), status);
                 if (U_FAILURE(status)) {
                     fSkeletons.adoptInstead(nullptr);
                     return;
                 }
-                newElem.orphan(); // fSkeletons vector now owns the UnicodeString.
+                newElem.orphan(); // fSkeletons vector now owns the UnicodeString (although it
+                                  // does not use a deleter function to manage the ownership).
             }
             curElem = curElem->next.getAlias();
         }
@@ -2865,12 +2866,13 @@ DTRedundantEnumeration::add(const UnicodeString& pattern, UErrorCode& status) {
     if (U_FAILURE(status)) {
         return;
     }
-    fPatterns->addElementX(newElem.getAlias(), status);
+    fPatterns->addElement(newElem.getAlias(), status);
     if (U_FAILURE(status)) {
         fPatterns.adoptInstead(nullptr);
         return;
     }
-    newElem.orphan(); // fPatterns now owns the string.
+    newElem.orphan(); // fPatterns now owns the string, although a UVector
+                      // deleter function is not used to manage that ownership.
 }
 
 const UnicodeString*

diff --git a/icu4c/source/i18n/msgfmt.cpp b/icu4c/source/i18n/msgfmt.cpp
index b8cb2e2ca560fe881949761154c7e5ef405b66ac..13a5a0895160fb69cbd6d2ef23bf26928ba04a5e 100644 (file)
--- a/icu4c/source/i18n/msgfmt.cpp
+++ b/icu4c/source/i18n/msgfmt.cpp
@@ -854,19 +854,21 @@ StringEnumeration*
 MessageFormat::getFormatNames(UErrorCode& status) {
     if (U_FAILURE(status))  return NULL;
 
-    UVector *fFormatNames = new UVector(status);
+    LocalPointer<UVector> formatNames(new UVector(status), status);
     if (U_FAILURE(status)) {
-        status = U_MEMORY_ALLOCATION_ERROR;
-        return NULL;
+        return nullptr;
     }
-    fFormatNames->setDeleter(uprv_deleteUObject);
+    formatNames->setDeleter(uprv_deleteUObject);
 
     for (int32_t partIndex = 0; (partIndex = nextTopLevelArgStart(partIndex)) >= 0;) {
-        fFormatNames->addElementX(new UnicodeString(getArgName(partIndex + 1)), status);
+        LocalPointer<UnicodeString> name(getArgName(partIndex + 1).clone(), status);
+        formatNames->adoptElement(name.orphan(), status);
+        if (U_FAILURE(status))  return nullptr;
     }
 
-    StringEnumeration* nameEnumerator = new FormatNameEnumeration(fFormatNames, status);
-    return nameEnumerator;
+    LocalPointer<StringEnumeration> nameEnumerator(
+        new FormatNameEnumeration(std::move(formatNames), status), status);
+    return U_SUCCESS(status) ? nameEnumerator.orphan() : nullptr;
 }
 
 // -------------------------------------
@@ -1912,9 +1914,9 @@ void MessageFormat::DummyFormat::parseObject(const UnicodeString&,
 }
 
 
-FormatNameEnumeration::FormatNameEnumeration(UVector *fNameList, UErrorCode& /*status*/) {
+FormatNameEnumeration::FormatNameEnumeration(LocalPointer<UVector> nameList, UErrorCode& /*status*/) {
     pos=0;
-    fFormatNames = fNameList;
+    fFormatNames = std::move(nameList);
 }
 
 const UnicodeString*
@@ -1936,7 +1938,6 @@ FormatNameEnumeration::count(UErrorCode& /*status*/) const {
 }
 
 FormatNameEnumeration::~FormatNameEnumeration() {
-    delete fFormatNames;
 }
 
 MessageFormat::PluralSelectorProvider::PluralSelectorProvider(const MessageFormat &mf, UPluralType t)

diff --git a/icu4c/source/i18n/msgfmt_impl.h b/icu4c/source/i18n/msgfmt_impl.h
index 57988389132a67b6a3a41b524e6cf979b6100c80..80cb8a691eb1ddd4d7a9cfd1c6ad9060d2a17f97 100644 (file)
--- a/icu4c/source/i18n/msgfmt_impl.h
+++ b/icu4c/source/i18n/msgfmt_impl.h
@@ -26,7 +26,7 @@ U_NAMESPACE_BEGIN
 
 class FormatNameEnumeration : public StringEnumeration {
 public:
-    FormatNameEnumeration(UVector *fFormatNames, UErrorCode& status);
+    FormatNameEnumeration(LocalPointer<UVector> fFormatNames, UErrorCode& status);
     virtual ~FormatNameEnumeration();
     static UClassID U_EXPORT2 getStaticClassID(void);
     virtual UClassID getDynamicClassID(void) const override;
@@ -35,7 +35,7 @@ public:
     virtual int32_t count(UErrorCode& status) const override;
 private:
     int32_t pos;
-    UVector *fFormatNames;
+    LocalPointer<UVector> fFormatNames;
 };
 
 U_NAMESPACE_END

diff --git a/icu4c/source/i18n/number_compact.cpp b/icu4c/source/i18n/number_compact.cpp
index 62692f444dff071a34a956807db428f27754684f..60cd7bedf667b0a5318a8b2212049d2bf7ce4474 100644 (file)
--- a/icu4c/source/i18n/number_compact.cpp
+++ b/icu4c/source/i18n/number_compact.cpp
@@ -157,8 +157,8 @@ void CompactData::getUniquePatterns(UVector &output, UErrorCode &status) const {
         }
 
         // The string was not found; add it to the UVector.
-        // ANDY: This requires a const_cast.  Why?
-        output.addElementX(const_cast<UChar *>(pattern), status);
+        // Note: must cast off const from pattern to store it in a UVector, which expects (void *)
+        output.addElement(const_cast<UChar *>(pattern), status);
 
         continue_outer:
         continue;

diff --git a/icu4c/source/i18n/numsys.cpp b/icu4c/source/i18n/numsys.cpp
index 44aaf8e2a5f9878d066969cc7d371053d23607e7..934149039c52d7365dcca90054d4431fb13bd78e 100644 (file)
--- a/icu4c/source/i18n/numsys.cpp
+++ b/icu4c/source/i18n/numsys.cpp
@@ -313,12 +313,7 @@ U_CFUNC void initNumsysNames(UErrorCode &status) {
         }
         const char *nsName = ures_getKey(nsCurrent.getAlias());
         LocalPointer<UnicodeString> newElem(new UnicodeString(nsName, -1, US_INV), status);
-        if (U_SUCCESS(status)) {
-            numsysNames->addElementX(newElem.getAlias(), status);
-            if (U_SUCCESS(status)) {
-                newElem.orphan(); // on success, the numsysNames vector owns newElem.
-            }
-        }
+        numsysNames->adoptElement(newElem.orphan(), status);
     }
 
     ures_close(numberingSystemsInfo);

diff --git a/icu4c/source/i18n/plurrule.cpp b/icu4c/source/i18n/plurrule.cpp
index d1918c4698138b96fe7adae7f1f921d49cfe1f36..cb817042da83d4a4db09a5b8586c2b17b8b5f5e8 100644 (file)
--- a/icu4c/source/i18n/plurrule.cpp
+++ b/icu4c/source/i18n/plurrule.cpp
@@ -1548,14 +1548,9 @@ PluralKeywordEnumeration::PluralKeywordEnumeration(RuleChain *header, UErrorCode
     UBool  addKeywordOther = TRUE;
     RuleChain *node = header;
     while (node != nullptr) {
-        auto newElem = new UnicodeString(node->fKeyword);
-        if (newElem == nullptr) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-            return;
-        }
-        fKeywordNames.addElementX(newElem, status);
+        LocalPointer<UnicodeString> newElem(node->fKeyword.clone(), status);
+        fKeywordNames.adoptElement(newElem.orphan(), status);
         if (U_FAILURE(status)) {
-            delete newElem;
             return;
         }
         if (0 == node->fKeyword.compare(PLURAL_KEYWORD_OTHER, 5)) {
@@ -1565,14 +1560,9 @@ PluralKeywordEnumeration::PluralKeywordEnumeration(RuleChain *header, UErrorCode
     }
 
     if (addKeywordOther) {
-        auto newElem = new UnicodeString(PLURAL_KEYWORD_OTHER);
-        if (newElem == nullptr) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-            return;
-        }
-        fKeywordNames.addElementX(newElem, status);
+        LocalPointer<UnicodeString> newElem(new UnicodeString(PLURAL_KEYWORD_OTHER), status);
+        fKeywordNames.adoptElement(newElem.orphan(), status);
         if (U_FAILURE(status)) {
-            delete newElem;
             return;
         }
     }

diff --git a/icu4c/source/i18n/tmutfmt.cpp b/icu4c/source/i18n/tmutfmt.cpp
index 057bb634ebbb2458cbfee03d6f014e0437e0955f..f0335a81f50fa4e4eaefdd6aa4632262439cda58 100644 (file)
--- a/icu4c/source/i18n/tmutfmt.cpp
+++ b/icu4c/source/i18n/tmutfmt.cpp
@@ -320,14 +320,14 @@ void
 TimeUnitFormat::setup(UErrorCode& err) {
     initDataMembers(err);
 
-    UVector pluralCounts(0, uhash_compareUnicodeString, 6, err);
+    UVector pluralCounts(nullptr, uhash_compareUnicodeString, 6, err);
     LocalPointer<StringEnumeration> keywords(getPluralRules().getKeywords(err), err);
     if (U_FAILURE(err)) {
         return;
     }
     UnicodeString* pluralCount;
     while ((pluralCount = const_cast<UnicodeString*>(keywords->snext(err))) != NULL) {
-      pluralCounts.addElementX(pluralCount, err);
+      pluralCounts.addElement(pluralCount, err);
     }
     readFromCurrentLocale(UTMUTFMT_FULL_STYLE, gUnitsTag, pluralCounts, err);
     checkConsistency(UTMUTFMT_FULL_STYLE, gUnitsTag, err);