ICU-20041 Improve handling of OOM failures in NumberingSystem class. (#19)

ICU-20041 ICU4C NumberingSystem class doesn't handle out-of-memory (OOM) failures.

- Not all code paths in the NumberingSystem class check for OOM failures. This can lead to crashes in some cases as null pointers will be dereferenced without any checks.
- Change to use nullptr instead of NULL.
- Don't stomp on OOM errors when attempting to load resources. We should report back OOM to the caller.
- Use LocalPointer in order simplify the code and for automatic clean-up of memory.
- Use LocalUResourceBundlePointer as well to help simply things even more.
This commit is contained in:
Jeff Genovy 2018-08-02 16:23:07 -07:00 committed by Shane Carr
parent 3712aa3d6e
commit d07396f94d
No known key found for this signature in database
GPG Key ID: FCED3B24AAB18B5C
2 changed files with 88 additions and 60 deletions

View File

@ -79,43 +79,45 @@ NumberingSystem* U_EXPORT2
NumberingSystem::createInstance(int32_t radix_in, UBool isAlgorithmic_in, const UnicodeString & desc_in, UErrorCode &status) {
if (U_FAILURE(status)) {
return NULL;
return nullptr;
}
if ( radix_in < 2 ) {
status = U_ILLEGAL_ARGUMENT_ERROR;
return NULL;
return nullptr;
}
if ( !isAlgorithmic_in ) {
if ( desc_in.countChar32() != radix_in ) {
status = U_ILLEGAL_ARGUMENT_ERROR;
return NULL;
return nullptr;
}
}
NumberingSystem *ns = new NumberingSystem();
LocalPointer<NumberingSystem> ns(new NumberingSystem(), status);
if (U_FAILURE(status)) {
return nullptr;
}
ns->setRadix(radix_in);
ns->setDesc(desc_in);
ns->setAlgorithmic(isAlgorithmic_in);
ns->setName(NULL);
return ns;
}
ns->setName(nullptr);
return ns.orphan();
}
NumberingSystem* U_EXPORT2
NumberingSystem::createInstance(const Locale & inLocale, UErrorCode& status) {
if (U_FAILURE(status)) {
return NULL;
return nullptr;
}
UBool nsResolved = TRUE;
UBool usingFallback = FALSE;
char buffer[ULOC_KEYWORDS_CAPACITY];
int32_t count = inLocale.getKeywordValue("numbers",buffer, sizeof(buffer),status);
int32_t count = inLocale.getKeywordValue("numbers", buffer, sizeof(buffer), status);
if (U_FAILURE(status) || status == U_STRING_NOT_TERMINATED_WARNING) {
// the "numbers" keyword exceeds ULOC_KEYWORDS_CAPACITY; ignore and use default.
count = 0;
@ -129,20 +131,30 @@ NumberingSystem::createInstance(const Locale & inLocale, UErrorCode& status) {
nsResolved = FALSE;
}
} else {
uprv_strcpy(buffer,gDefault);
uprv_strcpy(buffer, gDefault);
nsResolved = FALSE;
}
if (!nsResolved) { // Resolve the numbering system ( default, native, traditional or finance ) into a "real" numbering system
UErrorCode localStatus = U_ZERO_ERROR;
UResourceBundle *resource = ures_open(NULL, inLocale.getName(), &localStatus);
UResourceBundle *numberElementsRes = ures_getByKey(resource,gNumberElements,NULL,&localStatus);
LocalUResourceBundlePointer resource(ures_open(nullptr, inLocale.getName(), &localStatus));
LocalUResourceBundlePointer numberElementsRes(ures_getByKey(resource.getAlias(), gNumberElements, nullptr, &localStatus));
// Don't stomp on the catastrophic failure of OOM.
if (localStatus == U_MEMORY_ALLOCATION_ERROR) {
status = U_MEMORY_ALLOCATION_ERROR;
return nullptr;
}
while (!nsResolved) {
localStatus = U_ZERO_ERROR;
count = 0;
const UChar *nsName = ures_getStringByKeyWithFallback(numberElementsRes, buffer, &count, &localStatus);
const UChar *nsName = ures_getStringByKeyWithFallback(numberElementsRes.getAlias(), buffer, &count, &localStatus);
// Don't stomp on the catastrophic failure of OOM.
if (localStatus == U_MEMORY_ALLOCATION_ERROR) {
status = U_MEMORY_ALLOCATION_ERROR;
return nullptr;
}
if ( count > 0 && count < ULOC_KEYWORDS_CAPACITY ) { // numbering system found
u_UCharsToChars(nsName,buffer,count);
u_UCharsToChars(nsName, buffer, count);
buffer[count] = '\0'; // Make sure it is null terminated.
nsResolved = TRUE;
}
@ -158,16 +170,17 @@ NumberingSystem::createInstance(const Locale & inLocale, UErrorCode& status) {
}
}
}
ures_close(numberElementsRes);
ures_close(resource);
}
if (usingFallback) {
status = U_USING_FALLBACK_WARNING;
NumberingSystem *ns = new NumberingSystem();
if (ns == nullptr) {
status = U_MEMORY_ALLOCATION_ERROR;
}
return ns;
} else {
return NumberingSystem::createInstanceByName(buffer,status);
return NumberingSystem::createInstanceByName(buffer, status);
}
}
@ -178,36 +191,37 @@ NumberingSystem::createInstance(UErrorCode& status) {
NumberingSystem* U_EXPORT2
NumberingSystem::createInstanceByName(const char *name, UErrorCode& status) {
UResourceBundle *numberingSystemsInfo = NULL;
UResourceBundle *nsTop, *nsCurrent;
int32_t radix = 10;
int32_t algorithmic = 0;
numberingSystemsInfo = ures_openDirect(NULL,gNumberingSystems, &status);
nsCurrent = ures_getByKey(numberingSystemsInfo,gNumberingSystems,NULL,&status);
nsTop = ures_getByKey(nsCurrent,name,NULL,&status);
UnicodeString nsd = ures_getUnicodeStringByKey(nsTop,gDesc,&status);
LocalUResourceBundlePointer numberingSystemsInfo(ures_openDirect(nullptr, gNumberingSystems, &status));
LocalUResourceBundlePointer nsCurrent(ures_getByKey(numberingSystemsInfo.getAlias(), gNumberingSystems, nullptr, &status));
LocalUResourceBundlePointer nsTop(ures_getByKey(nsCurrent.getAlias(), name, nullptr, &status));
ures_getByKey(nsTop,gRadix,nsCurrent,&status);
radix = ures_getInt(nsCurrent,&status);
UnicodeString nsd = ures_getUnicodeStringByKey(nsTop.getAlias(), gDesc, &status);
ures_getByKey(nsTop,gAlgorithmic,nsCurrent,&status);
algorithmic = ures_getInt(nsCurrent,&status);
ures_getByKey(nsTop.getAlias(), gRadix, nsCurrent.getAlias(), &status);
radix = ures_getInt(nsCurrent.getAlias(), &status);
ures_getByKey(nsTop.getAlias(), gAlgorithmic, nsCurrent.getAlias(), &status);
algorithmic = ures_getInt(nsCurrent.getAlias(), &status);
UBool isAlgorithmic = ( algorithmic == 1 );
ures_close(nsCurrent);
ures_close(nsTop);
ures_close(numberingSystemsInfo);
if (U_FAILURE(status)) {
status = U_UNSUPPORTED_ERROR;
return NULL;
// Don't stomp on the catastrophic failure of OOM.
if (status != U_MEMORY_ALLOCATION_ERROR) {
status = U_UNSUPPORTED_ERROR;
}
return nullptr;
}
NumberingSystem* ns = NumberingSystem::createInstance(radix,isAlgorithmic,nsd,status);
LocalPointer<NumberingSystem> ns(NumberingSystem::createInstance(radix, isAlgorithmic, nsd, status), status);
if (U_FAILURE(status)) {
return nullptr;
}
ns->setName(name);
return ns;
return ns.orphan();
}
/**
@ -241,11 +255,11 @@ void NumberingSystem::setDesc(const UnicodeString &d) {
desc.setTo(d);
}
void NumberingSystem::setName(const char *n) {
if ( n == NULL ) {
if ( n == nullptr ) {
name[0] = (char) 0;
} else {
uprv_strncpy(name,n,NUMSYS_NAME_CAPACITY);
name[NUMSYS_NAME_CAPACITY] = (char)0; // Make sure it is null terminated.
name[NUMSYS_NAME_CAPACITY] = '\0'; // Make sure it is null terminated.
}
}
UBool NumberingSystem::isAlgorithmic() const {
@ -254,43 +268,57 @@ UBool NumberingSystem::isAlgorithmic() const {
StringEnumeration* NumberingSystem::getAvailableNames(UErrorCode &status) {
// TODO(ticket #11908): Init-once static cache, with u_cleanup() callback.
static StringEnumeration* availableNames = NULL;
static StringEnumeration* availableNames = nullptr;
if (U_FAILURE(status)) {
return NULL;
return nullptr;
}
if ( availableNames == NULL ) {
if ( availableNames == nullptr ) {
// TODO: Simple array of UnicodeString objects, based on length of table resource?
LocalPointer<UVector> numsysNames(new UVector(uprv_deleteUObject, NULL, status), status);
LocalPointer<UVector> numsysNames(new UVector(uprv_deleteUObject, nullptr, status), status);
if (U_FAILURE(status)) {
return NULL;
return nullptr;
}
UErrorCode rbstatus = U_ZERO_ERROR;
UResourceBundle *numberingSystemsInfo = ures_openDirect(NULL, "numberingSystems", &rbstatus);
numberingSystemsInfo = ures_getByKey(numberingSystemsInfo,"numberingSystems",numberingSystemsInfo,&rbstatus);
if(U_FAILURE(rbstatus)) {
status = U_MISSING_RESOURCE_ERROR;
UResourceBundle *numberingSystemsInfo = ures_openDirect(nullptr, "numberingSystems", &rbstatus);
numberingSystemsInfo = ures_getByKey(numberingSystemsInfo, "numberingSystems", numberingSystemsInfo, &rbstatus);
if (U_FAILURE(rbstatus)) {
// Don't stomp on the catastrophic failure of OOM.
if (rbstatus == U_MEMORY_ALLOCATION_ERROR) {
status = rbstatus;
} else {
status = U_MISSING_RESOURCE_ERROR;
}
ures_close(numberingSystemsInfo);
return NULL;
return nullptr;
}
while ( ures_hasNext(numberingSystemsInfo) ) {
UResourceBundle *nsCurrent = ures_getNextResource(numberingSystemsInfo,NULL,&rbstatus);
const char *nsName = ures_getKey(nsCurrent);
numsysNames->addElement(new UnicodeString(nsName, -1, US_INV),status);
ures_close(nsCurrent);
while ( ures_hasNext(numberingSystemsInfo) && U_SUCCESS(status) ) {
LocalUResourceBundlePointer nsCurrent(ures_getNextResource(numberingSystemsInfo, nullptr, &rbstatus));
if (rbstatus == U_MEMORY_ALLOCATION_ERROR) {
status = rbstatus; // we want to report OOM failure back to the caller.
break;
}
const char *nsName = ures_getKey(nsCurrent.getAlias());
LocalPointer<UnicodeString> newElem(new UnicodeString(nsName, -1, US_INV), status);
if (U_SUCCESS(status)) {
numsysNames->addElement(newElem.getAlias(), status);
if (U_SUCCESS(status)) {
newElem.orphan(); // on success, the numsysNames vector owns newElem.
}
}
}
ures_close(numberingSystemsInfo);
if (U_FAILURE(status)) {
return NULL;
return nullptr;
}
availableNames = new NumsysNameEnumeration(numsysNames.getAlias(), status);
if (availableNames == NULL) {
if (availableNames == nullptr) {
status = U_MEMORY_ALLOCATION_ERROR;
return NULL;
return nullptr;
}
numsysNames.orphan(); // The names got adopted.
}
@ -305,10 +333,10 @@ NumsysNameEnumeration::NumsysNameEnumeration(UVector *numsysNames, UErrorCode& /
const UnicodeString*
NumsysNameEnumeration::snext(UErrorCode& status) {
if (U_SUCCESS(status) && pos < fNumsysNames->size()) {
if (U_SUCCESS(status) && (fNumsysNames != nullptr) && (pos < fNumsysNames->size())) {
return (const UnicodeString*)fNumsysNames->elementAt(pos++);
}
return NULL;
return nullptr;
}
void
@ -318,7 +346,7 @@ NumsysNameEnumeration::reset(UErrorCode& /*status*/) {
int32_t
NumsysNameEnumeration::count(UErrorCode& /*status*/) const {
return (fNumsysNames==NULL) ? 0 : fNumsysNames->size();
return (fNumsysNames==nullptr) ? 0 : fNumsysNames->size();
}
NumsysNameEnumeration::~NumsysNameEnumeration() {

View File

@ -37,7 +37,7 @@ public:
virtual int32_t count(UErrorCode& status) const;
private:
int32_t pos;
UVector *fNumsysNames;
UVector *fNumsysNames = nullptr;
};
U_NAMESPACE_END