From 5b4ac1c77d9b1b52d12801fb749cc8cb5051fc79 Mon Sep 17 00:00:00 2001 From: Fredrik Roubert Date: Tue, 30 Oct 2018 22:12:07 +0100 Subject: [PATCH] ICU-20202 Replace char* kwdBuf with MemoryPool. Instead of _appendLDMLExtensionAsKeywords() requiring to receive a pre- allocated buffer of sufficient size to store all the temporary strings it needs to store, have it use a MemoryPool to allocate storage space as needed. Storing strings as individual CharString objects, instead of as NUL delimited substrings in a contiguous memory area, also eliminates the need for keeping track of string boundaries and NUL terminators. --- icu4c/source/common/uloc_tag.cpp | 78 ++++++++++++-------------------- 1 file changed, 30 insertions(+), 48 deletions(-) diff --git a/icu4c/source/common/uloc_tag.cpp b/icu4c/source/common/uloc_tag.cpp index 89702968cd7..5632a432bdd 100644 --- a/icu4c/source/common/uloc_tag.cpp +++ b/icu4c/source/common/uloc_tag.cpp @@ -1348,7 +1348,7 @@ cleanup: * Note: char* buf is used for storing keywords */ static void -_appendLDMLExtensionAsKeywords(const char* ldmlext, ExtensionListEntry** appendTo, char* buf, int32_t bufSize, UBool *posixVariant, UErrorCode *status) { +_appendLDMLExtensionAsKeywords(const char* ldmlext, ExtensionListEntry** appendTo, icu::MemoryPool& kwdBuf, UBool *posixVariant, UErrorCode *status) { const char *pTag; /* beginning of current subtag */ const char *pKwds; /* beginning of key-type pairs */ UBool variantExists = *posixVariant; @@ -1360,7 +1360,6 @@ _appendLDMLExtensionAsKeywords(const char* ldmlext, ExtensionListEntry** appendT AttributeListEntry *attr, *nextAttr; int32_t len; - int32_t bufIdx = 0; char attrBuf[ULOC_KEYWORD_AND_VALUES_CAPACITY]; int32_t attrBufIdx = 0; @@ -1416,40 +1415,34 @@ _appendLDMLExtensionAsKeywords(const char* ldmlext, ExtensionListEntry** appendT if (attrFirst) { /* emit attributes as an LDML keyword, e.g. attribute=attr1-attr2 */ - if (attrBufIdx > bufSize) { - /* attrBufIdx == + 1 */ - *status = U_ILLEGAL_ARGUMENT_ERROR; - goto cleanup; - } - kwd = (ExtensionListEntry*)uprv_malloc(sizeof(ExtensionListEntry)); if (kwd == NULL) { *status = U_MEMORY_ALLOCATION_ERROR; goto cleanup; } - kwd->key = LOCALE_ATTRIBUTE_KEY; - kwd->value = buf; + icu::CharString* value = kwdBuf.create(); + if (value == NULL) { + *status = U_MEMORY_ALLOCATION_ERROR; + goto cleanup; + } /* attribute subtags sorted in alphabetical order as type */ attr = attrFirst; while (attr != NULL) { nextAttr = attr->next; - - /* buffer size check is done above */ if (attr != attrFirst) { - *(buf + bufIdx) = SEP; - bufIdx++; + value->append('-', *status); } - - len = static_cast(uprv_strlen(attr->attribute)); - uprv_memcpy(buf + bufIdx, attr->attribute, len); - bufIdx += len; - + value->append(attr->attribute, *status); attr = nextAttr; } - *(buf + bufIdx) = 0; - bufIdx++; + if (U_FAILURE(*status)) { + goto cleanup; + } + + kwd->key = LOCALE_ATTRIBUTE_KEY; + kwd->value = value->data(); if (!_addExtensionToList(&kwdFirst, kwd, FALSE)) { *status = U_ILLEGAL_ARGUMENT_ERROR; @@ -1546,16 +1539,15 @@ _appendLDMLExtensionAsKeywords(const char* ldmlext, ExtensionListEntry** appendT We normalize the result key to lower case. */ T_CString_toLowerCase(bcpKeyBuf); - if (bufSize - bufIdx - 1 >= bcpKeyLen) { - uprv_memcpy(buf + bufIdx, bcpKeyBuf, bcpKeyLen); - pKey = buf + bufIdx; - bufIdx += bcpKeyLen; - *(buf + bufIdx) = 0; - bufIdx++; - } else { - *status = U_BUFFER_OVERFLOW_ERROR; + icu::CharString* key = kwdBuf.create(bcpKeyBuf, bcpKeyLen, *status); + if (key == NULL) { + *status = U_MEMORY_ALLOCATION_ERROR; goto cleanup; } + if (U_FAILURE(*status)) { + goto cleanup; + } + pKey = key->data(); } if (pBcpType) { @@ -1582,16 +1574,15 @@ _appendLDMLExtensionAsKeywords(const char* ldmlext, ExtensionListEntry** appendT */ /* normalize to lower case */ T_CString_toLowerCase(bcpTypeBuf); - if (bufSize - bufIdx - 1 >= bcpTypeLen) { - uprv_memcpy(buf + bufIdx, bcpTypeBuf, bcpTypeLen); - pType = buf + bufIdx; - bufIdx += bcpTypeLen; - *(buf + bufIdx) = 0; - bufIdx++; - } else { - *status = U_BUFFER_OVERFLOW_ERROR; + icu::CharString* type = kwdBuf.create(bcpTypeBuf, bcpTypeLen, *status); + if (type == NULL) { + *status = U_MEMORY_ALLOCATION_ERROR; goto cleanup; } + if (U_FAILURE(*status)) { + goto cleanup; + } + pType = type->data(); } } else { /* typeless - default type value is "yes" */ @@ -1662,20 +1653,13 @@ _appendKeywords(ULanguageTag* langtag, char* appendAt, int32_t capacity, UErrorC ExtensionListEntry *kwdFirst = NULL; ExtensionListEntry *kwd; const char *key, *type; - char *kwdBuf = NULL; - int32_t kwdBufLength = capacity; + icu::MemoryPool kwdBuf; UBool posixVariant = FALSE; if (U_FAILURE(*status)) { return 0; } - kwdBuf = (char*)uprv_malloc(kwdBufLength); - if (kwdBuf == NULL) { - *status = U_MEMORY_ALLOCATION_ERROR; - return 0; - } - /* Determine if variants already exists */ if (ultag_getVariantsSize(langtag)) { posixVariant = TRUE; @@ -1688,7 +1672,7 @@ _appendKeywords(ULanguageTag* langtag, char* appendAt, int32_t capacity, UErrorC key = ultag_getExtensionKey(langtag, i); type = ultag_getExtensionValue(langtag, i); if (*key == LDMLEXT) { - _appendLDMLExtensionAsKeywords(type, &kwdFirst, kwdBuf, kwdBufLength, &posixVariant, status); + _appendLDMLExtensionAsKeywords(type, &kwdFirst, kwdBuf, &posixVariant, status); if (U_FAILURE(*status)) { break; } @@ -1785,8 +1769,6 @@ _appendKeywords(ULanguageTag* langtag, char* appendAt, int32_t capacity, UErrorC kwd = tmpKwd; } - uprv_free(kwdBuf); - if (U_FAILURE(*status)) { return 0; }