Revise uses of UVector in rbtz.cpp 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 include
- Use LocalPointers and UVector deleter functions to simplify OOM checking and recovery.
- Fix RuleBasedTimeZone::addTransitionRule(rule) to have standard ICU adopt behavior
when errors occur, meaning automatic deletion of the incoming rule. This simplifies
both the implementation of the function and the code at the call sites.
- Update addTransitionRule() call sites. Includes modifying the Dangi calendar initializtion
to not silently ignore errors.
- struct Transition is changed to derive from UMemory, which allows the use of LocalPointers.
//-------------------------------------------------------------------------
DangiCalendar::DangiCalendar(const Locale& aLocale, UErrorCode& success)
-: ChineseCalendar(aLocale, DANGI_EPOCH_YEAR, getDangiCalZoneAstroCalc(), success)
+: ChineseCalendar(aLocale, DANGI_EPOCH_YEAR, getDangiCalZoneAstroCalc(success), success)
{
}
* 1898-1911: GMT+8
* 1912- : GMT+9
*/
-static void U_CALLCONV initDangiCalZoneAstroCalc(void) {
- U_ASSERT(gDangiCalendarZoneAstroCalc == NULL);
+static void U_CALLCONV initDangiCalZoneAstroCalc(UErrorCode &status) {
+ U_ASSERT(gDangiCalendarZoneAstroCalc == nullptr);
const UDate millis1897[] = { (UDate)((1897 - 1970) * 365 * kOneDay) }; // some days of error is not a problem here
const UDate millis1898[] = { (UDate)((1898 - 1970) * 365 * kOneDay) }; // some days of error is not a problem here
const UDate millis1912[] = { (UDate)((1912 - 1970) * 365 * kOneDay) }; // this doesn't create an issue for 1911/12/20
- InitialTimeZoneRule* initialTimeZone = new InitialTimeZoneRule(UNICODE_STRING_SIMPLE("GMT+8"), 8*kOneHour, 0);
- TimeZoneRule* rule1897 = new TimeArrayTimeZoneRule(UNICODE_STRING_SIMPLE("Korean 1897"), 7*kOneHour, 0, millis1897, 1, DateTimeRule::STANDARD_TIME);
- TimeZoneRule* rule1898to1911 = new TimeArrayTimeZoneRule(UNICODE_STRING_SIMPLE("Korean 1898-1911"), 8*kOneHour, 0, millis1898, 1, DateTimeRule::STANDARD_TIME);
- TimeZoneRule* ruleFrom1912 = new TimeArrayTimeZoneRule(UNICODE_STRING_SIMPLE("Korean 1912-"), 9*kOneHour, 0, millis1912, 1, DateTimeRule::STANDARD_TIME);
- UErrorCode status = U_ZERO_ERROR;
- RuleBasedTimeZone* dangiCalZoneAstroCalc = new RuleBasedTimeZone(UNICODE_STRING_SIMPLE("KOREA_ZONE"), initialTimeZone); // adopts initialTimeZone
- dangiCalZoneAstroCalc->addTransitionRule(rule1897, status); // adopts rule1897
- dangiCalZoneAstroCalc->addTransitionRule(rule1898to1911, status);
- dangiCalZoneAstroCalc->addTransitionRule(ruleFrom1912, status);
+ LocalPointer<InitialTimeZoneRule> initialTimeZone(new InitialTimeZoneRule(
+ UnicodeString(u"GMT+8"), 8*kOneHour, 0), status);
+
+ LocalPointer<TimeZoneRule> rule1897(new TimeArrayTimeZoneRule(
+ UnicodeString(u"Korean 1897"), 7*kOneHour, 0, millis1897, 1, DateTimeRule::STANDARD_TIME), status);
+
+ LocalPointer<TimeZoneRule> rule1898to1911(new TimeArrayTimeZoneRule(
+ UnicodeString(u"Korean 1898-1911"), 8*kOneHour, 0, millis1898, 1, DateTimeRule::STANDARD_TIME), status);
+
+ LocalPointer<TimeZoneRule> ruleFrom1912(new TimeArrayTimeZoneRule(
+ UnicodeString(u"Korean 1912-"), 9*kOneHour, 0, millis1912, 1, DateTimeRule::STANDARD_TIME), status);
+
+ LocalPointer<RuleBasedTimeZone> dangiCalZoneAstroCalc(new RuleBasedTimeZone(
+ UnicodeString(u"KOREA_ZONE"), initialTimeZone.orphan()), status); // adopts initialTimeZone
+
+ if (U_FAILURE(status)) {
+ return;
+ }
+ dangiCalZoneAstroCalc->addTransitionRule(rule1897.orphan(), status); // adopts rule1897
+ dangiCalZoneAstroCalc->addTransitionRule(rule1898to1911.orphan(), status);
+ dangiCalZoneAstroCalc->addTransitionRule(ruleFrom1912.orphan(), status);
dangiCalZoneAstroCalc->complete(status);
if (U_SUCCESS(status)) {
- gDangiCalendarZoneAstroCalc = dangiCalZoneAstroCalc;
- } else {
- delete dangiCalZoneAstroCalc;
- gDangiCalendarZoneAstroCalc = NULL;
+ gDangiCalendarZoneAstroCalc = dangiCalZoneAstroCalc.orphan();
}
ucln_i18n_registerCleanup(UCLN_I18N_DANGI_CALENDAR, calendar_dangi_cleanup);
}
-const TimeZone* DangiCalendar::getDangiCalZoneAstroCalc(void) const {
- umtx_initOnce(gDangiCalendarInitOnce, &initDangiCalZoneAstroCalc);
+const TimeZone* DangiCalendar::getDangiCalZoneAstroCalc(UErrorCode &status) const {
+ umtx_initOnce(gDangiCalendarInitOnce, &initDangiCalZoneAstroCalc, status);
return gDangiCalendarZoneAstroCalc;
}
private:
- const TimeZone* getDangiCalZoneAstroCalc(void) const;
+ const TimeZone* getDangiCalZoneAstroCalc(UErrorCode &status) const;
// UObject stuff
public:
/**
* A struct representing a time zone transition
*/
-struct Transition {
+struct Transition : public UMemory {
UDate time;
TimeZoneRule* from;
TimeZoneRule* to;
};
+U_CDECL_BEGIN
+static void U_CALLCONV
+deleteTransition(void* obj) {
+ delete static_cast<Transition *>(obj);
+}
+U_CDECL_END
+
static UBool compareRules(UVector* rules1, UVector* rules2) {
if (rules1 == NULL && rules2 == NULL) {
return TRUE;
void
RuleBasedTimeZone::addTransitionRule(TimeZoneRule* rule, UErrorCode& status) {
+ LocalPointer<TimeZoneRule>lpRule(rule);
if (U_FAILURE(status)) {
return;
}
AnnualTimeZoneRule* atzrule = dynamic_cast<AnnualTimeZoneRule*>(rule);
- if (atzrule != NULL && atzrule->getEndYear() == AnnualTimeZoneRule::MAX_YEAR) {
+ if (atzrule != nullptr && atzrule->getEndYear() == AnnualTimeZoneRule::MAX_YEAR) {
// A final rule
- if (fFinalRules == NULL) {
- fFinalRules = new UVector(status);
+ if (fFinalRules == nullptr) {
+ LocalPointer<UVector> lpFinalRules(new UVector(uprv_deleteUObject, nullptr, status), status);
if (U_FAILURE(status)) {
return;
}
+ fFinalRules = lpFinalRules.orphan();
} else if (fFinalRules->size() >= 2) {
// Cannot handle more than two final rules
status = U_INVALID_STATE_ERROR;
return;
}
- fFinalRules->addElementX((void*)rule, status);
+ fFinalRules->adoptElement(lpRule.orphan(), status);
} else {
// Non-final rule
- if (fHistoricRules == NULL) {
- fHistoricRules = new UVector(status);
+ if (fHistoricRules == nullptr) {
+ LocalPointer<UVector> lpHistoricRules(new UVector(uprv_deleteUObject, nullptr, status), status);
if (U_FAILURE(status)) {
return;
}
+ fHistoricRules = lpHistoricRules.orphan();
}
- fHistoricRules->addElementX((void*)rule, status);
+ fHistoricRules->adoptElement(lpRule.orphan(), status);
}
// Mark dirty, so transitions are recalculated at next complete() call
fUpToDate = FALSE;
return;
}
- UBool *done = NULL;
// Create a TimezoneTransition and add to the list
if (fHistoricRules != NULL || fFinalRules != NULL) {
TimeZoneRule *curRule = fInitialRule;
if (fHistoricRules != NULL && fHistoricRules->size() > 0) {
int32_t i;
int32_t historicCount = fHistoricRules->size();
- done = (UBool*)uprv_malloc(sizeof(UBool) * historicCount);
+ LocalMemory<bool> done((bool *)uprv_malloc(sizeof(bool) * historicCount));
if (done == NULL) {
status = U_MEMORY_ALLOCATION_ERROR;
goto cleanup;
}
for (i = 0; i < historicCount; i++) {
- done[i] = FALSE;
+ done[i] = false;
}
while (TRUE) {
int32_t curStdOffset = curRule->getRawOffset();
avail = r->getNextStart(lastTransitionTime, curStdOffset, curDstSavings, false, tt);
if (!avail) {
// No more transitions from this rule - skip this rule next time
- done[i] = TRUE;
+ done[i] = true;
} else {
r->getName(name);
if (*r == *curRule ||
}
if (fHistoricTransitions == NULL) {
- fHistoricTransitions = new UVector(status);
+ LocalPointer<UVector> lpHistoricTransitions(
+ new UVector(deleteTransition, nullptr, status), status);
if (U_FAILURE(status)) {
goto cleanup;
}
+ fHistoricTransitions = lpHistoricTransitions.orphan();
}
- Transition *trst = (Transition*)uprv_malloc(sizeof(Transition));
- if (trst == NULL) {
- status = U_MEMORY_ALLOCATION_ERROR;
+ LocalPointer<Transition> trst(new Transition, status);
+ if (U_FAILURE(status)) {
goto cleanup;
}
trst->time = nextTransitionTime;
trst->from = curRule;
trst->to = nextRule;
- fHistoricTransitions->addElementX(trst, status);
+ fHistoricTransitions->adoptElement(trst.orphan(), status);
if (U_FAILURE(status)) {
goto cleanup;
}
}
if (fFinalRules != NULL) {
if (fHistoricTransitions == NULL) {
- fHistoricTransitions = new UVector(status);
+ LocalPointer<UVector> lpHistoricTransitions(
+ new UVector(deleteTransition, nullptr, status), status);
if (U_FAILURE(status)) {
goto cleanup;
}
+ fHistoricTransitions = lpHistoricTransitions.orphan();
}
// Append the first transition for each
TimeZoneRule *rule0 = (TimeZoneRule*)fFinalRules->elementAt(0);
status = U_INVALID_STATE_ERROR;
goto cleanup;
}
- Transition *final0 = (Transition*)uprv_malloc(sizeof(Transition));
- if (final0 == NULL) {
- status = U_MEMORY_ALLOCATION_ERROR;
- goto cleanup;
- }
- Transition *final1 = (Transition*)uprv_malloc(sizeof(Transition));
- if (final1 == NULL) {
- uprv_free(final0);
- status = U_MEMORY_ALLOCATION_ERROR;
- goto cleanup;
+ LocalPointer<Transition> final0(new Transition, status);
+ LocalPointer<Transition> final1(new Transition, status);
+ if (U_FAILURE(status)) {
+ goto cleanup;
}
if (tt0 < tt1) {
final0->time = tt0;
final1->from = rule1;
final1->to = rule0;
}
- fHistoricTransitions->addElementX(final0, status);
- if (U_FAILURE(status)) {
- goto cleanup;
- }
- fHistoricTransitions->addElementX(final1, status);
+ fHistoricTransitions->adoptElement(final0.orphan(), status);
+ fHistoricTransitions->adoptElement(final1.orphan(), status);
if (U_FAILURE(status)) {
goto cleanup;
}
}
}
fUpToDate = TRUE;
- if (done != NULL) {
- uprv_free(done);
- }
return;
cleanup:
deleteTransitions();
- if (done != NULL) {
- uprv_free(done);
- }
fUpToDate = FALSE;
}
delete fInitialRule;
fInitialRule = NULL;
if (fHistoricRules != NULL) {
- while (!fHistoricRules->isEmpty()) {
- delete (TimeZoneRule*)(fHistoricRules->orphanElementAt(0));
- }
delete fHistoricRules;
fHistoricRules = NULL;
}
if (fFinalRules != NULL) {
- while (!fFinalRules->isEmpty()) {
- delete (AnnualTimeZoneRule*)(fFinalRules->orphanElementAt(0));
- }
delete fFinalRules;
fFinalRules = NULL;
}
void
RuleBasedTimeZone::deleteTransitions(void) {
if (fHistoricTransitions != NULL) {
- while (!fHistoricTransitions->isEmpty()) {
- Transition *trs = (Transition*)fHistoricTransitions->orphanElementAt(0);
- uprv_free(trs);
- }
delete fHistoricTransitions;
}
fHistoricTransitions = NULL;
UVector*
RuleBasedTimeZone::copyRules(UVector* source) {
- if (source == NULL) {
- return NULL;
+ if (source == nullptr) {
+ return nullptr;
}
UErrorCode ec = U_ZERO_ERROR;
int32_t size = source->size();
- UVector *rules = new UVector(size, ec);
+ LocalPointer<UVector> rules(new UVector(uprv_deleteUObject, nullptr, size, ec), ec);
if (U_FAILURE(ec)) {
- return NULL;
+ return nullptr;
}
int32_t i;
for (i = 0; i < size; i++) {
- rules->addElementX(((TimeZoneRule*)source->elementAt(i))->clone(), ec);
+ LocalPointer<TimeZoneRule> rule(((TimeZoneRule*)source->elementAt(i))->clone(), ec);
+ rules->adoptElement(rule.orphan(), ec);
if (U_FAILURE(ec)) {
- break;
+ return nullptr;
}
}
- if (U_FAILURE(ec)) {
- // In case of error, clean up
- for (i = 0; i < rules->size(); i++) {
- TimeZoneRule *rule = (TimeZoneRule*)rules->orphanElementAt(i);
- delete rule;
- }
- delete rules;
- return NULL;
- }
- return rules;
+ return rules.orphan();
}
TimeZoneRule*
virtual bool operator!=(const TimeZone& that) const;
/**
- * Adds the <code>TimeZoneRule</code> which represents time transitions.
- * The <code>TimeZoneRule</code> must have start times, that is, the result
- * of isTransitionRule() must be true. Otherwise, U_ILLEGAL_ARGUMENT_ERROR
+ * Adds the `TimeZoneRule` which represents time transitions.
+ * The `TimeZoneRule` must have start times, that is, the result
+ * of `isTransitionRule()` must be true. Otherwise, U_ILLEGAL_ARGUMENT_ERROR
* is set to the error code.
- * The input <code>TimeZoneRule</code> is adopted by this
- * <code>RuleBasedTimeZone</code> on successful completion of this method,
- * thus, the caller must not delete it when no error is returned.
- * After all rules are added, the caller must call complete() method to
- * make this <code>RuleBasedTimeZone</code> ready to handle common time
+ * The input `TimeZoneRule` is adopted by this `RuleBasedTimeZone`;
+ * the caller must not delete it. Should an error condition prevent
+ * the successful adoption of the rule, this function will delete it.
+ *
+ * After all rules are added, the caller must call `complete()` method to
+ * make this `RuleBasedTimeZone` ready to handle common time
* zone functions.
- * @param rule The <code>TimeZoneRule</code>.
+ * @param rule The `TimeZoneRule`.
* @param status Output param to filled in with a success or an error.
* @stable ICU 3.8
*/
InitialTimeZoneRule *initial = nullptr;
AnnualTimeZoneRule *std = nullptr, *dst = nullptr;
getSimpleRulesNear(time, initial, std, dst, status);
+ LocalPointer<InitialTimeZoneRule> lpInitial(initial);
+ LocalPointer<AnnualTimeZoneRule> lpStd(std);
+ LocalPointer<AnnualTimeZoneRule> lpDst(dst);
if (U_SUCCESS(status)) {
// Create a RuleBasedTimeZone with the subset rule
getID(tzid);
- RuleBasedTimeZone rbtz(tzid, initial);
- if (std != nullptr && dst != nullptr) {
- rbtz.addTransitionRule(std, status);
- rbtz.addTransitionRule(dst, status);
+ RuleBasedTimeZone rbtz(tzid, lpInitial.orphan());
+ if (lpStd.isValid() && lpDst.isValid()) {
+ rbtz.addTransitionRule(lpStd.orphan(), status);
+ rbtz.addTransitionRule(lpDst.orphan(), status);
}
if (U_FAILURE(status)) {
- goto cleanupWriteSimple;
+ return;
}
if (olsonzid.length() > 0 && icutzver.length() > 0) {
- UnicodeString *icutzprop = new UnicodeString(ICU_TZINFO_PROP);
- if (icutzprop == nullptr) {
- status = U_MEMORY_ALLOCATION_ERROR;
- goto cleanupWriteSimple;
+ LocalPointer<UnicodeString> icutzprop(new UnicodeString(ICU_TZINFO_PROP), status);
+ if (U_FAILURE(status)) {
+ return;
}
icutzprop->append(olsonzid);
icutzprop->append((UChar)0x005B/*'['*/);
icutzprop->append(ICU_TZINFO_SIMPLE, -1);
appendMillis(time, *icutzprop);
icutzprop->append((UChar)0x005D/*']'*/);
- customProps.addElementX(icutzprop, status);
- if (U_FAILURE(status)) {
- delete icutzprop;
- goto cleanupWriteSimple;
- }
+ customProps.adoptElement(icutzprop.orphan(), status);
}
writeZone(writer, rbtz, &customProps, status);
}
- return;
-
-cleanupWriteSimple:
- if (initial != nullptr) {
- delete initial;
- }
- if (std != nullptr) {
- delete std;
- }
- if (dst != nullptr) {
- delete dst;
- }
}
void
rbtz1->addTransitionRule(atzr, status);
if (U_SUCCESS(status)) {
errln("FAIL: 3rd final rule must be rejected");
- } else {
- delete atzr;
}
// Try to add an initial rule
rbtz1->addTransitionRule(ir1, status);
if (U_SUCCESS(status)) {
errln("FAIL: InitialTimeZoneRule must be rejected");
- } else {
- delete ir1;
}
delete ir;
projects / icu / commitdiff