organicmaps/icu
13
Fork 0
mirror of https://github.com/unicode-org/icu.git synced 2025-04-08 06:53:45 +00:00
Code Issues 1 Projects Releases Wiki Activity Actions 2

ICU-21662 UVector cleanup in rbtz.cpp

Browse source 
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.
This commit is contained in:
Andy Heninger 2021-09-15 14:51:48 -07:00
parent b2c1dffb70
commit 35dff23fb5
6 changed files with 94 additions and 123 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
icu4c/source/i18n/dangical.cpp
View file

@ -52,7 +52,7 @@ U_NAMESPACE_BEGIN
//-------------------------------------------------------------------------
DangiCalendar::DangiCalendar(const Locale& aLocale, UErrorCode& success)
: ChineseCalendar(aLocale, DANGI_EPOCH_YEAR, getDangiCalZoneAstroCalc(), success)
: ChineseCalendar(aLocale, DANGI_EPOCH_YEAR, getDangiCalZoneAstroCalc(success), success)
{
}
@ -103,32 +103,41 @@ const char *DangiCalendar::getType() const {
* 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;
}

2
icu4c/source/i18n/dangical.h
View file

@ -74,7 +74,7 @@ class DangiCalendar : public ChineseCalendar {
private:
const TimeZone* getDangiCalZoneAstroCalc(void) const;
const TimeZone* getDangiCalZoneAstroCalc(UErrorCode &status) const;
// UObject stuff
public:

109
icu4c/source/i18n/rbtz.cpp
View file

@ -25,12 +25,19 @@ U_NAMESPACE_BEGIN
/**
* 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;
@ -114,32 +121,35 @@ RuleBasedTimeZone::operator!=(const TimeZone& that) const {
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;
@ -175,7 +185,6 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
return;
}
UBool *done = NULL;
// Create a TimezoneTransition and add to the list
if (fHistoricRules != NULL || fFinalRules != NULL) {
TimeZoneRule *curRule = fInitialRule;
@ -186,13 +195,13 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
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();
@ -213,7 +222,7 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
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 ||
@ -266,20 +275,21 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
}
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;
}
@ -289,10 +299,12 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
}
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);
@ -305,16 +317,10 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
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;
@ -331,27 +337,18 @@ RuleBasedTimeZone::complete(UErrorCode& status) {
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;
}
@ -628,16 +625,10 @@ RuleBasedTimeZone::deleteRules(void) {
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;
}
@ -646,10 +637,6 @@ RuleBasedTimeZone::deleteRules(void) {
void
RuleBasedTimeZone::deleteTransitions(void) {
if (fHistoricTransitions != NULL) {
while (!fHistoricTransitions->isEmpty()) {
Transition *trs = (Transition*)fHistoricTransitions->orphanElementAt(0);
uprv_free(trs);
}
delete fHistoricTransitions;
}
fHistoricTransitions = NULL;
@ -657,32 +644,24 @@ RuleBasedTimeZone::deleteTransitions(void) {
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*

19
icu4c/source/i18n/unicode/rbtz.h
View file

@ -89,17 +89,18 @@ public:
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
*/

38
icu4c/source/i18n/vtzone.cpp
View file

@ -1894,23 +1894,25 @@ VTimeZone::writeSimple(UDate time, VTZWriter& writer, UErrorCode& status) const
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/*'['*/);
@ -1918,26 +1920,10 @@ VTimeZone::writeSimple(UDate time, VTZWriter& writer, UErrorCode& status) const
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

4
icu4c/source/test/intltest/tzrulets.cpp
View file

@ -402,8 +402,6 @@ TimeZoneRuleTest::TestSimpleRuleBasedTimeZone(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
@ -411,8 +409,6 @@ TimeZoneRuleTest::TestSimpleRuleBasedTimeZone(void) {
rbtz1->addTransitionRule(ir1, status);
if (U_SUCCESS(status)) {
errln("FAIL: InitialTimeZoneRule must be rejected");
} else {
delete ir1;
}
delete ir;