From 1d398369bd5f2bed7c5895cdbe987a4547ae1128 Mon Sep 17 00:00:00 2001 From: Jeff Genovy <29107334+jefgen@users.noreply.github.com> Date: Thu, 2 Aug 2018 00:09:14 -0700 Subject: [PATCH] ICU-20044 Fix some OOM issues in the NumberFormat class. (#21) --- icu4c/source/i18n/numfmt.cpp | 50 +++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/icu4c/source/i18n/numfmt.cpp b/icu4c/source/i18n/numfmt.cpp index cf0ce6f0ed5..626a5d16fb2 100644 --- a/icu4c/source/i18n/numfmt.cpp +++ b/icu4c/source/i18n/numfmt.cpp @@ -1326,13 +1326,13 @@ NumberFormat::makeInstance(const Locale& desiredLocale, // if the locale has "@compat=host", create a host-specific NumberFormat if (U_SUCCESS(status) && count > 0 && uprv_strcmp(buffer, "host") == 0) { - Win32NumberFormat *f = NULL; UBool curr = TRUE; switch (style) { case UNUM_DECIMAL: curr = FALSE; // fall-through + U_FALLTHROUGH; case UNUM_CURRENCY: case UNUM_CURRENCY_ISO: // do not support plural formatting here @@ -1340,14 +1340,13 @@ NumberFormat::makeInstance(const Locale& desiredLocale, case UNUM_CURRENCY_ACCOUNTING: case UNUM_CASH_CURRENCY: case UNUM_CURRENCY_STANDARD: - f = new Win32NumberFormat(desiredLocale, curr, status); - + { + LocalPointer f(new Win32NumberFormat(desiredLocale, curr, status), status); if (U_SUCCESS(status)) { - return f; + return f.orphan(); } - - delete f; - break; + } + break; default: break; } @@ -1417,8 +1416,7 @@ NumberFormat::makeInstance(const Locale& desiredLocale, } } - - NumberFormat *f; + LocalPointer f; if (ns->isAlgorithmic()) { UnicodeString nsDesc; UnicodeString nsRuleSetGroup; @@ -1453,7 +1451,7 @@ NumberFormat::makeInstance(const Locale& desiredLocale, return NULL; } r->setDefaultRuleSet(nsRuleSetName,status); - f = r; + f.adoptInstead(r); } else { // replace single currency sign in the pattern with double currency sign // if the style is UNUM_CURRENCY_ISO @@ -1462,9 +1460,22 @@ NumberFormat::makeInstance(const Locale& desiredLocale, UnicodeString(TRUE, gDoubleCurrencySign, 2)); } - // "new DecimalFormat()" does not adopt the symbols if its memory allocation fails. - DecimalFormatSymbols *syms = symbolsToAdopt.orphan(); - DecimalFormat* df = new DecimalFormat(pattern, syms, style, status); + // "new DecimalFormat()" does not adopt the symbols argument if its memory allocation fails. + // So we can't use adoptInsteadAndCheckErrorCode as we need to know if the 'new' failed. + DecimalFormatSymbols *syms = symbolsToAdopt.getAlias(); + LocalPointer df(new DecimalFormat(pattern, syms, style, status)); + + if (df.isValid()) { + // if the DecimalFormat object was successfully new'ed, then it will own symbolsToAdopt, even if the status is a failure. + symbolsToAdopt.orphan(); + } + else { + status = U_MEMORY_ALLOCATION_ERROR; + } + + if (U_FAILURE(status)) { + return nullptr; + } // if it is cash currency style, setCurrencyUsage with usage if (style == UNUM_CASH_CURRENCY){ @@ -1472,25 +1483,18 @@ NumberFormat::makeInstance(const Locale& desiredLocale, } if (U_FAILURE(status)) { - delete df; - return NULL; + return nullptr; } - f = df; - if (f == NULL) { - delete syms; - status = U_MEMORY_ALLOCATION_ERROR; - return NULL; - } + f.adoptInstead(df.orphan()); } f->setLocaleIDs(ures_getLocaleByType(ownedResource.getAlias(), ULOC_VALID_LOCALE, &status), ures_getLocaleByType(ownedResource.getAlias(), ULOC_ACTUAL_LOCALE, &status)); if (U_FAILURE(status)) { - delete f; return NULL; } - return f; + return f.orphan(); } /**