ICU-21763 UVector cleanup in Locale & Region Code

Revise uses of UVector in Locale and Region 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.

In Region::loadRegionData(), improved the overall error detection and recovery.
This commit is contained in:
Andy Heninger 2021-11-09 12:53:59 -08:00
parent d3a56c5ced
commit c48f218600
4 changed files with 108 additions and 110 deletions

View file

@ -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<UVector> 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<Locale> 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<Locale> clone(locale.clone(), errorCode_);
supportedLocales_->adoptElement(clone.orphan(), errorCode_);
}
return *this;
}

View file

@ -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<CharString> 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; }

View file

@ -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<UVector> continents(new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status);
LocalPointer<UVector> groupings(new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status);
allRegions = new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status);
LocalPointer<UVector> 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<UnicodeString> newRegion(new UnicodeString(buf), status);
allRegions->addElementX(newRegion.orphan(),status);
allRegions->adoptElement(newRegion.orphan(), status);
buf[rangeMarkerLocation-1]++;
}
} else {
LocalPointer<UnicodeString> 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<UnicodeString> newRegion(new UnicodeString(buf), status);
allRegions->addElementX(newRegion.orphan(),status);
allRegions->adoptElement(newRegion.orphan(),status);
buf[rangeMarkerLocation-1]++;
}
} else {
LocalPointer<UnicodeString> newRegion(new UnicodeString(regionName), status);
allRegions->addElementX(newRegion.orphan(),status);
allRegions->adoptElement(newRegion.orphan(),status);
}
}
while ( ures_hasNext(regionUnknown.getAlias()) ) {
LocalPointer<UnicodeString> regionName (new UnicodeString(ures_getNextUnicodeString(regionUnknown.getAlias(),NULL,&status),status));
allRegions->addElementX(regionName.orphan(),status);
while (U_SUCCESS(status) && ures_hasNext(regionUnknown.getAlias())) {
LocalPointer<UnicodeString> 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<UnicodeString> 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<UVector> 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<UnicodeString> 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<UnicodeString> 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 *)&currentRegion);
if (target) {
LocalPointer<UnicodeString> 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<UVector> lpContainedRegions(
new UVector(uprv_deleteUObject, uhash_compareUnicodeString, status), status);
parentRegion->containedRegions = lpContainedRegions.orphan();
if (U_FAILURE(status)) {
return;
}
}
LocalPointer<UnicodeString> 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<StringEnumeration> 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<UnicodeString *>(&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<StringEnumeration> 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<UnicodeString *>(&r2->idStr), status);
}
delete children;
}
}
delete cr;
StringEnumeration* resultEnumeration = new RegionNameEnumeration(result,status);
delete result;
return resultEnumeration;
LocalPointer<StringEnumeration> 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<UVector> 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<UnicodeString> 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;
}
}

View file

@ -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;