diff --git a/icu4c/source/common/localematcher.cpp b/icu4c/source/common/localematcher.cpp index 3d178dfbaf1..2cad708d99f 100644 --- a/icu4c/source/common/localematcher.cpp +++ b/icu4c/source/common/localematcher.cpp @@ -168,12 +168,9 @@ void LocaleMatcher::Builder::clearSupportedLocales() { bool LocaleMatcher::Builder::ensureSupportedLocaleVector() { if (U_FAILURE(errorCode_)) { return false; } if (supportedLocales_ != nullptr) { return true; } - supportedLocales_ = new UVector(uprv_deleteUObject, nullptr, errorCode_); + LocalPointer lpSupportedLocales(new UVector(uprv_deleteUObject, nullptr, errorCode_), errorCode_); if (U_FAILURE(errorCode_)) { return false; } - if (supportedLocales_ == nullptr) { - errorCode_ = U_MEMORY_ALLOCATION_ERROR; - return false; - } + supportedLocales_ = lpSupportedLocales.orphan(); return true; } @@ -187,9 +184,8 @@ LocaleMatcher::Builder &LocaleMatcher::Builder::setSupportedLocalesFromListStrin for (int32_t i = 0; i < length; ++i) { Locale *locale = list.orphanLocaleAt(i); if (locale == nullptr) { continue; } - supportedLocales_->addElementX(locale, errorCode_); + supportedLocales_->adoptElement(locale, errorCode_); if (U_FAILURE(errorCode_)) { - delete locale; break; } } @@ -197,35 +193,21 @@ LocaleMatcher::Builder &LocaleMatcher::Builder::setSupportedLocalesFromListStrin } LocaleMatcher::Builder &LocaleMatcher::Builder::setSupportedLocales(Locale::Iterator &locales) { - if (U_FAILURE(errorCode_)) { return *this; } - clearSupportedLocales(); - if (!ensureSupportedLocaleVector()) { return *this; } - while (locales.hasNext()) { - const Locale &locale = locales.next(); - Locale *clone = locale.clone(); - if (clone == nullptr) { - errorCode_ = U_MEMORY_ALLOCATION_ERROR; - break; - } - supportedLocales_->addElementX(clone, errorCode_); - if (U_FAILURE(errorCode_)) { - delete clone; - break; + if (ensureSupportedLocaleVector()) { + clearSupportedLocales(); + while (locales.hasNext() && U_SUCCESS(errorCode_)) { + const Locale &locale = locales.next(); + LocalPointer clone (locale.clone(), errorCode_); + supportedLocales_->adoptElement(clone.orphan(), errorCode_); } } return *this; } LocaleMatcher::Builder &LocaleMatcher::Builder::addSupportedLocale(const Locale &locale) { - if (!ensureSupportedLocaleVector()) { return *this; } - Locale *clone = locale.clone(); - if (clone == nullptr) { - errorCode_ = U_MEMORY_ALLOCATION_ERROR; - return *this; - } - supportedLocales_->addElementX(clone, errorCode_); - if (U_FAILURE(errorCode_)) { - delete clone; + if (ensureSupportedLocaleVector()) { + LocalPointer clone(locale.clone(), errorCode_); + supportedLocales_->adoptElement(clone.orphan(), errorCode_); } return *this; } diff --git a/icu4c/source/common/locid.cpp b/icu4c/source/common/locid.cpp index e8859c7048b..73bb8d8aec1 100644 --- a/icu4c/source/common/locid.cpp +++ b/icu4c/source/common/locid.cpp @@ -1204,14 +1204,11 @@ AliasReplacer::parseLanguageReplacement( // We have multiple field so we have to allocate and parse CharString* str = new CharString( replacement, (int32_t)uprv_strlen(replacement), status); + LocalPointer lpStr(str, status); + toBeFreed.adoptElement(lpStr.orphan(), status); if (U_FAILURE(status)) { return; } - if (str == nullptr) { - status = U_MEMORY_ALLOCATION_ERROR; - return; - } - toBeFreed.addElementX(str, status); char* data = str->data(); replacedLanguage = (const char*) data; char* endOfField = uprv_strchr(data, '_'); @@ -1420,12 +1417,9 @@ AliasReplacer::replaceTerritory(UVector& toBeFreed, UErrorCode& status) (int32_t)(firstSpace - replacement), status), status); } if (U_FAILURE(status)) { return false; } - if (item.isNull()) { - status = U_MEMORY_ALLOCATION_ERROR; - return false; - } replacedRegion = item->data(); - toBeFreed.addElementX(item.orphan(), status); + toBeFreed.adoptElement(item.orphan(), status); + if (U_FAILURE(status)) { return false; } } U_ASSERT(!same(region, replacedRegion)); region = replacedRegion; @@ -1659,10 +1653,10 @@ AliasReplacer::replace(const Locale& locale, CharString& out, UErrorCode& status while ((end = uprv_strchr(start, SEP_CHAR)) != nullptr && U_SUCCESS(status)) { *end = NULL_CHAR; // null terminate inside variantsBuff - variants.addElementX(start, status); + variants.addElement(start, status); start = end + 1; } - variants.addElementX(start, status); + variants.addElement(start, status); } if (U_FAILURE(status)) { return false; } diff --git a/icu4c/source/i18n/region.cpp b/icu4c/source/i18n/region.cpp index 2e013708bb8..277a22fd091 100644 --- a/icu4c/source/i18n/region.cpp +++ b/icu4c/source/i18n/region.cpp @@ -39,11 +39,6 @@ U_CDECL_BEGIN -static void U_CALLCONV -deleteRegion(void *obj) { - delete (icu::Region *)obj; -} - /** * Cleanup callback func */ @@ -90,7 +85,8 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { LocalPointer continents(new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status); LocalPointer groupings(new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status); - allRegions = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status); + LocalPointer lpAllRegions(new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status); + allRegions = lpAllRegions.orphan(); LocalUResourceBundlePointer metadata(ures_openDirect(NULL,"metadata",&status)); LocalUResourceBundlePointer metadataAlias(ures_getByKey(metadata.getAlias(),"alias",NULL,&status)); @@ -109,16 +105,17 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { LocalUResourceBundlePointer worldContainment(ures_getByKey(territoryContainment.getAlias(),"001",NULL,&status)); LocalUResourceBundlePointer groupingContainment(ures_getByKey(territoryContainment.getAlias(),"grouping",NULL,&status)); + ucln_i18n_registerCleanup(UCLN_I18N_REGION, region_cleanup); if (U_FAILURE(status)) { return; } // now, initialize - uhash_setValueDeleter(newRegionIDMap.getAlias(), deleteRegion); // regionIDMap owns objs - uhash_setKeyDeleter(newRegionAliases.getAlias(), uprv_deleteUObject); // regionAliases owns the string keys + uhash_setValueDeleter(newRegionIDMap.getAlias(), uprv_deleteUObject); // regionIDMap owns objs + uhash_setKeyDeleter(newRegionAliases.getAlias(), uprv_deleteUObject); // regionAliases owns the string keys - while ( ures_hasNext(regionRegular.getAlias()) ) { + while (U_SUCCESS(status) && ures_hasNext(regionRegular.getAlias())) { UnicodeString regionName = ures_getNextUnicodeString(regionRegular.getAlias(),NULL,&status); int32_t rangeMarkerLocation = regionName.indexOf(RANGE_MARKER); UChar buf[6]; @@ -126,18 +123,18 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { if ( rangeMarkerLocation > 0 ) { UChar endRange = regionName.charAt(rangeMarkerLocation+1); buf[rangeMarkerLocation] = 0; - while ( buf[rangeMarkerLocation-1] <= endRange ) { + while (U_SUCCESS(status) && buf[rangeMarkerLocation-1] <= endRange) { LocalPointer newRegion(new UnicodeString(buf), status); - allRegions->addElementX(newRegion.orphan(),status); + allRegions->adoptElement(newRegion.orphan(), status); buf[rangeMarkerLocation-1]++; } } else { LocalPointer newRegion(new UnicodeString(regionName), status); - allRegions->addElementX(newRegion.orphan(),status); + allRegions->adoptElement(newRegion.orphan(), status); } } - while ( ures_hasNext(regionMacro.getAlias()) ) { + while (U_SUCCESS(status) && ures_hasNext(regionMacro.getAlias())) { UnicodeString regionName = ures_getNextUnicodeString(regionMacro.getAlias(),NULL,&status); int32_t rangeMarkerLocation = regionName.indexOf(RANGE_MARKER); UChar buf[6]; @@ -145,25 +142,29 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { if ( rangeMarkerLocation > 0 ) { UChar endRange = regionName.charAt(rangeMarkerLocation+1); buf[rangeMarkerLocation] = 0; - while ( buf[rangeMarkerLocation-1] <= endRange ) { + while ( buf[rangeMarkerLocation-1] <= endRange && U_SUCCESS(status)) { LocalPointer newRegion(new UnicodeString(buf), status); - allRegions->addElementX(newRegion.orphan(),status); + allRegions->adoptElement(newRegion.orphan(),status); buf[rangeMarkerLocation-1]++; } } else { LocalPointer newRegion(new UnicodeString(regionName), status); - allRegions->addElementX(newRegion.orphan(),status); + allRegions->adoptElement(newRegion.orphan(),status); } } - while ( ures_hasNext(regionUnknown.getAlias()) ) { - LocalPointer regionName (new UnicodeString(ures_getNextUnicodeString(regionUnknown.getAlias(),NULL,&status),status)); - allRegions->addElementX(regionName.orphan(),status); + while (U_SUCCESS(status) && ures_hasNext(regionUnknown.getAlias())) { + LocalPointer regionName ( + new UnicodeString(ures_getNextUnicodeString(regionUnknown.getAlias(), nullptr, &status), status)); + allRegions->adoptElement(regionName.orphan(),status); } - while ( ures_hasNext(worldContainment.getAlias()) ) { + while (U_SUCCESS(status) && ures_hasNext(worldContainment.getAlias())) { UnicodeString *continentName = new UnicodeString(ures_getNextUnicodeString(worldContainment.getAlias(),NULL,&status)); - continents->addElementX(continentName,status); + continents->adoptElement(continentName,status); + } + if (U_FAILURE(status)) { + return; } for ( int32_t i = 0 ; i < allRegions->size() ; i++ ) { @@ -191,22 +192,32 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { } UResourceBundle *groupingBundle = nullptr; - while ( ures_hasNext(groupingContainment.getAlias()) ) { + while (U_SUCCESS(status) && ures_hasNext(groupingContainment.getAlias())) { groupingBundle = ures_getNextResource(groupingContainment.getAlias(), groupingBundle, &status); if (U_FAILURE(status)) { break; } UnicodeString *groupingName = new UnicodeString(ures_getKey(groupingBundle), -1, US_INV); - groupings->addElementX(groupingName,status); - Region *grouping = (Region *) uhash_get(newRegionIDMap.getAlias(),groupingName); + LocalPointer lpGroupingName(groupingName, status); + groupings->adoptElement(lpGroupingName.orphan(), status); + if (U_FAILURE(status)) { + break; + } + Region *grouping = (Region *) uhash_get(newRegionIDMap.getAlias(), groupingName); if (grouping != NULL) { - for (int32_t i = 0; i < ures_getSize(groupingBundle); i++) { + for (int32_t i = 0; i < ures_getSize(groupingBundle) && U_SUCCESS(status); i++) { UnicodeString child = ures_getUnicodeStringByIndex(groupingBundle, i, &status); if (U_SUCCESS(status)) { if (grouping->containedRegions == NULL) { - grouping->containedRegions = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status); + LocalPointer lpContainedRegions( + new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status); + grouping->containedRegions = lpContainedRegions.orphan(); + if (U_FAILURE(status)) { + break; + } } - grouping->containedRegions->addElementX(new UnicodeString(child), status); + LocalPointer lpChildCopy(new UnicodeString(child), status); + grouping->containedRegions->adoptElement(lpChildCopy.orphan(), status); } } } @@ -214,7 +225,7 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { ures_close(groupingBundle); // Process the territory aliases - while ( ures_hasNext(territoryAlias.getAlias()) ) { + while (U_SUCCESS(status) && ures_hasNext(territoryAlias.getAlias())) { LocalUResourceBundlePointer res(ures_getNextResource(territoryAlias.getAlias(),NULL,&status)); const char *aliasFrom = ures_getKey(res.getAlias()); LocalPointer aliasFromStr(new UnicodeString(aliasFrom, -1, US_INV), status); @@ -259,7 +270,7 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { } UnicodeString currentRegion; //currentRegion.remove(); TODO: was already 0 length? - for (int32_t i = 0 ; i < aliasTo.length() ; i++ ) { + for (int32_t i = 0 ; i < aliasTo.length() && U_SUCCESS(status); i++ ) { if ( aliasTo.charAt(i) != 0x0020 ) { currentRegion.append(aliasTo.charAt(i)); } @@ -267,7 +278,7 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { Region *target = (Region *)uhash_get(newRegionIDMap.getAlias(),(void *)¤tRegion); if (target) { LocalPointer preferredValue(new UnicodeString(target->idStr), status); - aliasFromRegion->preferredValues->addElementX((void *)preferredValue.orphan(),status); // may add null if err + aliasFromRegion->preferredValues->adoptElement(preferredValue.orphan(),status); // may add null if err } currentRegion.remove(); } @@ -276,9 +287,9 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { } // Process the code mappings - This will allow us to assign numeric codes to most of the territories. - while ( ures_hasNext(codeMappings.getAlias()) ) { + while (U_SUCCESS(status) && ures_hasNext(codeMappings.getAlias())) { UResourceBundle *mapping = ures_getNextResource(codeMappings.getAlias(),NULL,&status); - if ( ures_getType(mapping) == URES_ARRAY && ures_getSize(mapping) == 3) { + if (U_SUCCESS(status) && ures_getType(mapping) == URES_ARRAY && ures_getSize(mapping) == 3) { UnicodeString codeMappingID = ures_getUnicodeStringByIndex(mapping,0,&status); UnicodeString codeMappingNumber = ures_getUnicodeStringByIndex(mapping,1,&status); UnicodeString codeMapping3Letter = ures_getUnicodeStringByIndex(mapping,2,&status); @@ -356,15 +367,23 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { // Add the child region to the set of regions contained by the parent if (parentRegion->containedRegions == NULL) { - parentRegion->containedRegions = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status); + LocalPointer lpContainedRegions( + new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status); + parentRegion->containedRegions = lpContainedRegions.orphan(); + if (U_FAILURE(status)) { + return; + } } LocalPointer childStr(new UnicodeString(), status); - if( U_FAILURE(status) ) { + if (U_FAILURE(status)) { return; // error out } childStr->fastCopyFrom(childRegion->idStr); - parentRegion->containedRegions->addElementX((void *)childStr.orphan(),status); + parentRegion->containedRegions->adoptElement(childStr.orphan(),status); + if (U_FAILURE(status)) { + return; + } // Set the parent region to be the containing region of the child. // Regions of type GROUPING can't be set as the parent, since another region @@ -388,10 +407,9 @@ void U_CALLCONV Region::loadRegionData(UErrorCode &status) { if( U_FAILURE(status) ) { return; // error out } - availableRegions[ar->fType]->addElementX((void *)arString.orphan(),status); + availableRegions[ar->fType]->adoptElement(arString.orphan(), status); } - ucln_i18n_registerCleanup(UCLN_I18N_REGION, region_cleanup); // copy hashtables numericCodeMap = newNumericCodeMap.orphan(); regionIDMap = newRegionIDMap.orphan(); @@ -402,6 +420,7 @@ void Region::cleanupRegionData() { for (int32_t i = 0 ; i < URGN_LIMIT ; i++ ) { if ( availableRegions[i] ) { delete availableRegions[i]; + availableRegions[i] = nullptr; } } @@ -417,7 +436,6 @@ void Region::cleanupRegionData() { uhash_close(regionIDMap); } if (allRegions) { - allRegions->removeAllElements(); // Don't need the temporary list anymore. delete allRegions; allRegions = NULL; } @@ -615,33 +633,30 @@ Region::getContainedRegions(UErrorCode &status) const { StringEnumeration* Region::getContainedRegions( URegionType type, UErrorCode &status ) const { umtx_initOnce(gRegionDataInitOnce, &loadRegionData, status); // returns immediately if U_FAILURE(status) + + UVector result(nullptr, uhash_compareChars, status); + LocalPointer cr(getContainedRegions(status), status); if (U_FAILURE(status)) { - return NULL; + return nullptr; } - UVector *result = new UVector(NULL, uhash_compareChars, status); - - StringEnumeration *cr = getContainedRegions(status); - - for ( int32_t i = 0 ; i < cr->count(status) ; i++ ) { - const char *regionId = cr->next(NULL,status); - const Region *r = Region::getInstance(regionId,status); + const char *regionId; + while((regionId = cr->next(nullptr, status)) != nullptr && U_SUCCESS(status)) { + const Region *r = Region::getInstance(regionId, status); if ( r->getType() == type) { - result->addElementX((void *)&r->idStr,status); + result.addElement(const_cast(&r->idStr), status); } else { - StringEnumeration *children = r->getContainedRegions(type, status); - for ( int32_t j = 0 ; j < children->count(status) ; j++ ) { - const char *id2 = children->next(NULL,status); + LocalPointer children(r->getContainedRegions(type, status)); + const char *id2; + while(U_SUCCESS(status) && ((id2 = children->next(nullptr, status)) != nullptr)) { const Region *r2 = Region::getInstance(id2,status); - result->addElementX((void *)&r2->idStr,status); + result.addElement(const_cast(&r2->idStr), status); } - delete children; } } - delete cr; - StringEnumeration* resultEnumeration = new RegionNameEnumeration(result,status); - delete result; - return resultEnumeration; + LocalPointer resultEnumeration( + new RegionNameEnumeration(&result, status), status); + return U_SUCCESS(status) ? resultEnumeration.orphan() : nullptr; } /** @@ -706,18 +721,21 @@ Region::getType() const { return fType; } -RegionNameEnumeration::RegionNameEnumeration(UVector *fNameList, UErrorCode& status) { - pos=0; - if (fNameList && U_SUCCESS(status)) { - fRegionNames = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, fNameList->size(),status); - for ( int32_t i = 0 ; i < fNameList->size() ; i++ ) { - UnicodeString* this_region_name = (UnicodeString *)fNameList->elementAt(i); - UnicodeString* new_region_name = new UnicodeString(*this_region_name); - fRegionNames->addElementX((void *)new_region_name,status); +RegionNameEnumeration::RegionNameEnumeration(UVector *nameList, UErrorCode& status) : + pos(0), fRegionNames(nullptr) { + // TODO: https://unicode-org.atlassian.net/browse/ICU-21829 + // Is all of the copying going on here really necessary? + if (nameList && U_SUCCESS(status)) { + LocalPointer regionNames( + new UVector(uprv_deleteUObject, uhash_compareUnicodeString, nameList->size(), status), status); + for ( int32_t i = 0 ; U_SUCCESS(status) && i < nameList->size() ; i++ ) { + UnicodeString* this_region_name = (UnicodeString *)nameList->elementAt(i); + LocalPointer new_region_name(new UnicodeString(*this_region_name), status); + regionNames->adoptElement(new_region_name.orphan(), status); + } + if (U_SUCCESS(status)) { + fRegionNames = regionNames.orphan(); } - } - else { - fRegionNames = NULL; } } diff --git a/icu4c/source/i18n/region_impl.h b/icu4c/source/i18n/region_impl.h index 62acaa4511b..b6a281393f8 100644 --- a/icu4c/source/i18n/region_impl.h +++ b/icu4c/source/i18n/region_impl.h @@ -26,7 +26,11 @@ U_NAMESPACE_BEGIN class RegionNameEnumeration : public StringEnumeration { public: - RegionNameEnumeration(UVector *fNameList, UErrorCode& status); + /** + * Construct an string enumeration over the supplied name list. + * Makes a copy of the supplied input name list; does not retain a reference to the original. + */ + RegionNameEnumeration(UVector *nameList, UErrorCode& status); virtual ~RegionNameEnumeration(); static UClassID U_EXPORT2 getStaticClassID(void); virtual UClassID getDynamicClassID(void) const override;