From b82291fd77b0d425d06b6d39c769c0e58cddf9c4 Mon Sep 17 00:00:00 2001 From: Frank Tang Date: Thu, 6 Jun 2024 19:03:18 +0000 Subject: [PATCH] ICU-21810 Fix error checking of ucurr.cpp part 1 See #3027 --- icu4c/source/common/ucurr.cpp | 306 ++++++++++++---------------------- 1 file changed, 106 insertions(+), 200 deletions(-) diff --git a/icu4c/source/common/ucurr.cpp b/icu4c/source/common/ucurr.cpp index dbad1e501479..8c4db8cc5d57 100644 --- a/icu4c/source/common/ucurr.cpp +++ b/icu4c/source/common/ucurr.cpp @@ -306,10 +306,9 @@ _findMetaData(const char16_t* currency, UErrorCode& ec) { // move out of the root locale file later; if it does, update this // code.] UResourceBundle* currencyData = ures_openDirect(U_ICUDATA_CURR, CURRENCY_DATA, &ec); - UResourceBundle* currencyMeta = ures_getByKey(currencyData, CURRENCY_META, currencyData, &ec); + LocalUResourceBundlePointer currencyMeta(ures_getByKey(currencyData, CURRENCY_META, currencyData, &ec)); if (U_FAILURE(ec)) { - ures_close(currencyMeta); // Config/build error; return hard-coded defaults return LAST_RESORT_DATA; } @@ -317,32 +316,25 @@ _findMetaData(const char16_t* currency, UErrorCode& ec) { // Look up our currency, or if that's not available, then DEFAULT char buf[ISO_CURRENCY_CODE_LENGTH+1]; UErrorCode ec2 = U_ZERO_ERROR; // local error code: soft failure - UResourceBundle* rb = ures_getByKey(currencyMeta, myUCharsToChars(buf, currency), nullptr, &ec2); + LocalUResourceBundlePointer rb(ures_getByKey(currencyMeta.getAlias(), myUCharsToChars(buf, currency), nullptr, &ec2)); if (U_FAILURE(ec2)) { - ures_close(rb); - rb = ures_getByKey(currencyMeta,DEFAULT_META, nullptr, &ec); + rb.adoptInstead(ures_getByKey(currencyMeta.getAlias(),DEFAULT_META, nullptr, &ec)); if (U_FAILURE(ec)) { - ures_close(currencyMeta); - ures_close(rb); // Config/build error; return hard-coded defaults return LAST_RESORT_DATA; } } int32_t len; - const int32_t *data = ures_getIntVector(rb, &len, &ec); + const int32_t *data = ures_getIntVector(rb.getAlias(), &len, &ec); if (U_FAILURE(ec) || len != 4) { // Config/build error; return hard-coded defaults if (U_SUCCESS(ec)) { ec = U_INVALID_FORMAT_ERROR; } - ures_close(currencyMeta); - ures_close(rb); return LAST_RESORT_DATA; } - ures_close(currencyMeta); - ures_close(rb); return data; } @@ -565,14 +557,14 @@ ucurr_forLocale(const char* locale, localStatus = U_ZERO_ERROR; UResourceBundle *rb = ures_openDirect(U_ICUDATA_CURR, CURRENCY_DATA, &localStatus); UResourceBundle *cm = ures_getByKey(rb, CURRENCY_MAP, rb, &localStatus); - UResourceBundle *countryArray = ures_getByKey(rb, id.data(), cm, &localStatus); + LocalUResourceBundlePointer countryArray(ures_getByKey(rb, id.data(), cm, &localStatus)); // https://unicode-org.atlassian.net/browse/ICU-21997 // Prefer to use currencies that are legal tender. if (U_SUCCESS(localStatus)) { - int32_t arrayLength = ures_getSize(countryArray); + int32_t arrayLength = ures_getSize(countryArray.getAlias()); for (int32_t i = 0; i < arrayLength; ++i) { LocalUResourceBundlePointer currencyReq( - ures_getByIndex(countryArray, i, nullptr, &localStatus)); + ures_getByIndex(countryArray.getAlias(), i, nullptr, &localStatus)); // The currency is legal tender if it is *not* marked with tender{"false"}. UErrorCode tenderStatus = localStatus; const char16_t *tender = @@ -592,7 +584,6 @@ ucurr_forLocale(const char* locale, localStatus = U_MISSING_RESOURCE_ERROR; } } - ures_close(countryArray); } if ((U_FAILURE(localStatus)) && strchr(id.data(), '_') != nullptr) { @@ -805,21 +796,19 @@ ucurr_getPluralName(const char16_t* currency, rb = ures_getByKey(rb, CURRENCYPLURALS, rb, &ec2); // Fetch resource with multi-level resource inheritance fallback - rb = ures_getByKeyWithFallback(rb, buf, rb, &ec2); + LocalUResourceBundlePointer curr(ures_getByKeyWithFallback(rb, buf, rb, &ec2)); - s = ures_getStringByKeyWithFallback(rb, pluralCount, len, &ec2); + s = ures_getStringByKeyWithFallback(curr.getAlias(), pluralCount, len, &ec2); if (U_FAILURE(ec2)) { // fall back to "other" ec2 = U_ZERO_ERROR; - s = ures_getStringByKeyWithFallback(rb, "other", len, &ec2); + s = ures_getStringByKeyWithFallback(curr.getAlias(), "other", len, &ec2); if (U_FAILURE(ec2)) { - ures_close(rb); // fall back to long name in Currencies return ucurr_getName(currency, locale, UCURR_LONG_NAME, isChoiceFormat, len, ec); } } - ures_close(rb); // If we've succeeded we're done. Otherwise, try to fallback. // If that fails (because we are already at root) then exit. @@ -911,34 +900,29 @@ getCurrencyNameCount(const char* loc, int32_t* total_currency_name_count, int32_ for (;;) { UErrorCode ec2 = U_ZERO_ERROR; // TODO: ures_openDirect? - UResourceBundle* rb = ures_open(U_ICUDATA_CURR, locale.data(), &ec2); - UResourceBundle* curr = ures_getByKey(rb, CURRENCIES, nullptr, &ec2); - int32_t n = ures_getSize(curr); + LocalUResourceBundlePointer rb(ures_open(U_ICUDATA_CURR, locale.data(), &ec2)); + LocalUResourceBundlePointer curr(ures_getByKey(rb.getAlias(), CURRENCIES, nullptr, &ec2)); + int32_t n = ures_getSize(curr.getAlias()); for (int32_t i=0; i(symbol->getBuffer()); - (*currencySymbols)[*total_currency_symbol_count].flag = 0; - (*currencySymbols)[(*total_currency_symbol_count)++].currencyNameLen = symbol->length(); + (*currencySymbols)[(*total_currency_symbol_count)++] + = {iso, const_cast(symbol->getBuffer()), symbol->length(), 0}; } } // Add currency long name. - s = ures_getStringByIndex(names, UCURR_LONG_NAME, &len, &ec2); - (*currencyNames)[*total_currency_name_count].IsoCode = iso; + s = ures_getStringByIndex(names.getAlias(), UCURR_LONG_NAME, &len, &ec2); char16_t* upperName = toUpperCase(s, len, locale); - (*currencyNames)[*total_currency_name_count].currencyName = upperName; - (*currencyNames)[*total_currency_name_count].flag = NEED_TO_BE_DELETED; - (*currencyNames)[(*total_currency_name_count)++].currencyNameLen = len; + + (*currencyNames)[(*total_currency_name_count)++] = {iso, upperName, len, NEED_TO_BE_DELETED}; // put (iso, 3, and iso) in to array // Add currency ISO code. - (*currencySymbols)[*total_currency_symbol_count].IsoCode = iso; - (*currencySymbols)[*total_currency_symbol_count].currencyName = (char16_t*)uprv_malloc(sizeof(char16_t)*3); + char16_t* isoCode = (char16_t*)uprv_malloc(sizeof(char16_t)*3); // Must convert iso[] into Unicode - u_charsToUChars(iso, (*currencySymbols)[*total_currency_symbol_count].currencyName, 3); - (*currencySymbols)[*total_currency_symbol_count].flag = NEED_TO_BE_DELETED; - (*currencySymbols)[(*total_currency_symbol_count)++].currencyNameLen = 3; - - ures_close(names); + u_charsToUChars(iso, isoCode, 3); + (*currencySymbols)[(*total_currency_symbol_count)++] = {iso, isoCode, 3, NEED_TO_BE_DELETED}; } // currency plurals UErrorCode ec5 = U_ZERO_ERROR; - UResourceBundle* curr_p = ures_getByKey(rb, CURRENCYPLURALS, nullptr, &ec5); - n = ures_getSize(curr_p); + LocalUResourceBundlePointer curr_p(ures_getByKey(rb.getAlias(), CURRENCYPLURALS, nullptr, &ec5)); + n = ures_getSize(curr_p.getAlias()); for (int32_t i=0; iisoCode = isoCode; entry->from = fromDate; @@ -2161,13 +2104,10 @@ ucurr_createCurrencyList(UHashtable *isoCodes, UErrorCode* status){ } else { *status = localStatus; } - ures_close(currencyArray); } } else { *status = localStatus; } - - ures_close(currencyMapArray); } static const UEnumeration gEnumCurrencyList = { @@ -2186,19 +2126,18 @@ static void U_CALLCONV initIsoCodes(UErrorCode &status) { U_ASSERT(gIsoCodes == nullptr); ucln_common_registerCleanup(UCLN_COMMON_CURRENCY, currency_cleanup); - UHashtable *isoCodes = uhash_open(uhash_hashUChars, uhash_compareUChars, nullptr, &status); + LocalUHashtablePointer isoCodes(uhash_open(uhash_hashUChars, uhash_compareUChars, nullptr, &status)); if (U_FAILURE(status)) { return; } - uhash_setValueDeleter(isoCodes, deleteIsoCodeEntry); + uhash_setValueDeleter(isoCodes.getAlias(), deleteIsoCodeEntry); - ucurr_createCurrencyList(isoCodes, &status); + ucurr_createCurrencyList(isoCodes.getAlias(), &status); if (U_FAILURE(status)) { - uhash_close(isoCodes); return; } - gIsoCodes = isoCodes; // Note: gIsoCodes is const. Once set up here it is never altered, - // and read only access is safe without synchronization. + gIsoCodes = isoCodes.orphan(); // Note: gIsoCodes is const. Once set up here it is never altered, + // and read only access is safe without synchronization. } static void populateCurrSymbolsEquiv(icu::Hashtable *hash, UErrorCode &status) { @@ -2320,30 +2259,30 @@ ucurr_countCurrencies(const char* locale, UResourceBundle *cm = ures_getByKey(rb, CURRENCY_MAP, rb, &localStatus); // Using the id derived from the local, get the currency data - UResourceBundle *countryArray = ures_getByKey(rb, id.data(), cm, &localStatus); + LocalUResourceBundlePointer countryArray(ures_getByKey(rb, id.data(), cm, &localStatus)); // process each currency to see which one is valid for the given date if (U_SUCCESS(localStatus)) { - for (int32_t i=0; i 2) + if (ures_getSize(currencyRes.getAlias())> 2) { int32_t toLength = 0; - UResourceBundle *toRes = ures_getByKey(currencyRes, "to", nullptr, &localStatus); - const int32_t *toArray = ures_getIntVector(toRes, &toLength, &localStatus); + LocalUResourceBundlePointer toRes(ures_getByKey(currencyRes.getAlias(), "to", nullptr, &localStatus)); + const int32_t *toArray = ures_getIntVector(toRes.getAlias(), &toLength, &localStatus); currDate64 = (int64_t)toArray[0] << 32; currDate64 |= ((int64_t)toArray[1] & (int64_t)INT64_C(0x00000000FFFFFFFF)); @@ -2353,8 +2292,6 @@ ucurr_countCurrencies(const char* locale, { currCount++; } - - ures_close(toRes); } else { @@ -2363,16 +2300,9 @@ ucurr_countCurrencies(const char* locale, currCount++; } } - - // close open resources - ures_close(currencyRes); - ures_close(fromRes); - } // end For loop } // end if (U_SUCCESS(localStatus)) - ures_close(countryArray); - // Check for errors if (*ec == U_ZERO_ERROR || localStatus != U_ZERO_ERROR) { @@ -2386,7 +2316,6 @@ ucurr_countCurrencies(const char* locale, // no errors return currCount; } - } // If we got here, either error code is invalid or @@ -2433,39 +2362,38 @@ ucurr_forLocaleAndDate(const char* locale, UResourceBundle *cm = ures_getByKey(rb, CURRENCY_MAP, rb, &localStatus); // Using the id derived from the local, get the currency data - UResourceBundle *countryArray = ures_getByKey(rb, id.data(), cm, &localStatus); + LocalUResourceBundlePointer countryArray(ures_getByKey(rb, id.data(), cm, &localStatus)); // process each currency to see which one is valid for the given date bool matchFound = false; if (U_SUCCESS(localStatus)) { - if ((index <= 0) || (index> ures_getSize(countryArray))) + if ((index <= 0) || (index> ures_getSize(countryArray.getAlias()))) { // requested index is out of bounds - ures_close(countryArray); return 0; } - for (int32_t i=0; i 2) + if (ures_getSize(currencyRes.getAlias()) > 2) { int32_t toLength = 0; - UResourceBundle *toRes = ures_getByKey(currencyRes, "to", nullptr, &localStatus); - const int32_t *toArray = ures_getIntVector(toRes, &toLength, &localStatus); + LocalUResourceBundlePointer toRes(ures_getByKey(currencyRes.getAlias(), "to", nullptr, &localStatus)); + const int32_t *toArray = ures_getIntVector(toRes.getAlias(), &toLength, &localStatus); currDate64 = (int64_t)toArray[0] << 32; currDate64 |= ((int64_t)toArray[1] & (int64_t)INT64_C(0x00000000FFFFFFFF)); @@ -2479,8 +2407,6 @@ ucurr_forLocaleAndDate(const char* locale, matchFound = true; } } - - ures_close(toRes); } else { @@ -2493,11 +2419,6 @@ ucurr_forLocaleAndDate(const char* locale, } } } - - // close open resources - ures_close(currencyRes); - ures_close(fromRes); - // check for loop exit if (matchFound) { @@ -2507,8 +2428,6 @@ ucurr_forLocaleAndDate(const char* locale, } // end For loop } - ures_close(countryArray); - // Check for errors if (*ec == U_ZERO_ERROR || localStatus != U_ZERO_ERROR) { @@ -2578,20 +2497,16 @@ U_CAPI UEnumeration *U_EXPORT2 ucurr_getKeywordValuesForLocale(const char *key, memcpy(en, &defaultKeywordValues, sizeof(UEnumeration)); en->context = values; - UResourceBundle *bundle = ures_openDirect(U_ICUDATA_CURR, "supplementalData", status); - ures_getByKey(bundle, "CurrencyMap", bundle, status); - UResourceBundle bundlekey, regbndl, curbndl, to; - ures_initStackObject(&bundlekey); - ures_initStackObject(®bndl); - ures_initStackObject(&curbndl); - ures_initStackObject(&to); + UResourceBundle* rb = ures_openDirect(U_ICUDATA_CURR, "supplementalData", status); + LocalUResourceBundlePointer bundle(ures_getByKey(rb, "CurrencyMap", rb, status)); + StackUResourceBundle bundlekey, regbndl, curbndl, to; - while (U_SUCCESS(*status) && ures_hasNext(bundle)) { - ures_getNextResource(bundle, &bundlekey, status); + while (U_SUCCESS(*status) && ures_hasNext(bundle.getAlias())) { + ures_getNextResource(bundle.getAlias(), bundlekey.getAlias(), status); if (U_FAILURE(*status)) { break; } - const char *region = ures_getKey(&bundlekey); + const char *region = ures_getKey(bundlekey.getAlias()); UBool isPrefRegion = prefRegion == region; if (!isPrefRegion && commonlyUsed) { // With commonlyUsed=true, we do not put @@ -2599,29 +2514,29 @@ U_CAPI UEnumeration *U_EXPORT2 ucurr_getKeywordValuesForLocale(const char *key, // result list. continue; } - ures_getByKey(bundle, region, ®bndl, status); + ures_getByKey(bundle.getAlias(), region, regbndl.getAlias(), status); if (U_FAILURE(*status)) { break; } - while (U_SUCCESS(*status) && ures_hasNext(®bndl)) { - ures_getNextResource(®bndl, &curbndl, status); - if (ures_getType(&curbndl) != URES_TABLE) { + while (U_SUCCESS(*status) && ures_hasNext(regbndl.getAlias())) { + ures_getNextResource(regbndl.getAlias(), curbndl.getAlias(), status); + if (ures_getType(curbndl.getAlias()) != URES_TABLE) { // Currently, an empty ARRAY is mixed in. continue; } char *curID = (char *)uprv_malloc(sizeof(char) * ULOC_KEYWORDS_CAPACITY); - int32_t curIDLength = ULOC_KEYWORDS_CAPACITY; if (curID == nullptr) { *status = U_MEMORY_ALLOCATION_ERROR; break; } + int32_t curIDLength = ULOC_KEYWORDS_CAPACITY; #if U_CHARSET_FAMILY==U_ASCII_FAMILY - ures_getUTF8StringByKey(&curbndl, "id", curID, &curIDLength, true, status); + ures_getUTF8StringByKey(curbndl.getAlias(), "id", curID, &curIDLength, true, status); /* optimize - use the utf-8 string */ #else { - const char16_t* defString = ures_getStringByKey(&curbndl, "id", &curIDLength, status); + const char16_t* defString = ures_getStringByKey(curbndl.getAlias(), "id", &curIDLength, status); if(U_SUCCESS(*status)) { if(curIDLength+1 > ULOC_KEYWORDS_CAPACITY) { *status = U_BUFFER_OVERFLOW_ERROR; @@ -2636,7 +2551,7 @@ U_CAPI UEnumeration *U_EXPORT2 ucurr_getKeywordValuesForLocale(const char *key, break; } UBool hasTo = false; - ures_getByKey(&curbndl, "to", &to, status); + ures_getByKey(curbndl.getAlias(), "to", to.getAlias(), status); if (U_FAILURE(*status)) { // Do nothing here... *status = U_ZERO_ERROR; @@ -2677,7 +2592,6 @@ U_CAPI UEnumeration *U_EXPORT2 ucurr_getKeywordValuesForLocale(const char *key, } } } - ulist_resetList((UList *)(en->context)); } else { ulist_deleteList(values); @@ -2685,14 +2599,7 @@ U_CAPI UEnumeration *U_EXPORT2 ucurr_getKeywordValuesForLocale(const char *key, values = nullptr; en = nullptr; } - ures_close(&to); - ures_close(&curbndl); - ures_close(®bndl); - ures_close(&bundlekey); - ures_close(bundle); - ulist_deleteList(otherValues); - return en; } @@ -2703,19 +2610,18 @@ ucurr_getNumericCode(const char16_t* currency) { if (currency && u_strlen(currency) == ISO_CURRENCY_CODE_LENGTH) { UErrorCode status = U_ZERO_ERROR; - UResourceBundle *bundle = ures_openDirect(nullptr, "currencyNumericCodes", &status); - ures_getByKey(bundle, "codeMap", bundle, &status); + UResourceBundle* bundle = ures_openDirect(nullptr, "currencyNumericCodes", &status); + LocalUResourceBundlePointer codeMap(ures_getByKey(bundle, "codeMap", bundle, &status)); if (U_SUCCESS(status)) { char alphaCode[ISO_CURRENCY_CODE_LENGTH+1]; myUCharsToChars(alphaCode, currency); T_CString_toUpperCase(alphaCode); - ures_getByKey(bundle, alphaCode, bundle, &status); - int tmpCode = ures_getInt(bundle, &status); + ures_getByKey(codeMap.getAlias(), alphaCode, codeMap.getAlias(), &status); + int tmpCode = ures_getInt(codeMap.getAlias(), &status); if (U_SUCCESS(status)) { code = tmpCode; } } - ures_close(bundle); } return code; }