From a300d8dd029bd32133e111fc3a777ddf2515f1f0 Mon Sep 17 00:00:00 2001 From: Markus Scherer <markus.icu@gmail.com> Date: Thu, 26 Aug 2021 14:44:49 -0700 Subject: [PATCH] ICU-20769 refactor UResourceBundle.fResData & init_resb_result() - replace UResourceBundle.fResData with access via fData - change ResourceDataValue.resData into a pointer - move C tests to C++ that stack-allocate now-C++ UResourceBundle - init_resb_result(): - remove redundant ResourceData parameter - simplify memory management, and various small simplifications - /LOCALE/path reuse valid locale, no ures_open() - pull alias-fetching code into separate function getAliasTargetAsResourceBundle() - `const char *` for keyPath pieces - s/noAlias/recursionDepth/ - getAllItemsWithFallback() set correct parentBundle.fValidLocaleDataEntry --- icu4c/source/common/uresbund.cpp | 694 ++++++++++++------------ icu4c/source/common/uresdata.h | 11 +- icu4c/source/common/uresimp.h | 31 +- icu4c/source/test/cintltst/crestst.c | 43 -- icu4c/source/test/cintltst/creststn.c | 18 - icu4c/source/test/cintltst/creststn.h | 2 - icu4c/source/test/intltest/restsnew.cpp | 60 +- icu4c/source/test/intltest/restsnew.h | 3 + icu4c/source/tools/genrb/derb.cpp | 6 +- 9 files changed, 444 insertions(+), 424 deletions(-) diff --git a/icu4c/source/common/uresbund.cpp b/icu4c/source/common/uresbund.cpp index 4ca3a348910..5b2d89cf32c 100644 --- a/icu4c/source/common/uresbund.cpp +++ b/icu4c/source/common/uresbund.cpp @@ -113,47 +113,49 @@ static void entryIncrease(UResourceDataEntry *entry) { } /** - * Internal function. Tries to find a resource in given Resource + * Internal function. Tries to find a resource in given Resource * Bundle, as well as in its parents */ -static const ResourceData *getFallbackData(const UResourceBundle* resBundle, const char* * resTag, UResourceDataEntry* *realData, Resource *res, UErrorCode *status) { - UResourceDataEntry *resB = resBundle->fData; +static UResourceDataEntry *getFallbackData( + const UResourceBundle *resBundle, + const char **resTag, Resource *res, UErrorCode *status) { + UResourceDataEntry *dataEntry = resBundle->fData; int32_t indexR = -1; int32_t i = 0; *res = RES_BOGUS; - if(resB != NULL) { - if(resB->fBogus == U_ZERO_ERROR) { /* if this resource is real, */ - *res = res_getTableItemByKey(&(resB->fData), resB->fData.rootRes, &indexR, resTag); /* try to get data from there */ - i++; - } - if(resBundle->fHasFallback == TRUE) { - while(*res == RES_BOGUS && resB->fParent != NULL) { /* Otherwise, we'll look in parents */ - resB = resB->fParent; - if(resB->fBogus == U_ZERO_ERROR) { - i++; - *res = res_getTableItemByKey(&(resB->fData), resB->fData.rootRes, &indexR, resTag); - } + if(dataEntry == nullptr) { + *status = U_MISSING_RESOURCE_ERROR; + return nullptr; + } + if(dataEntry->fBogus == U_ZERO_ERROR) { /* if this resource is real, */ + *res = res_getTableItemByKey(&(dataEntry->fData), dataEntry->fData.rootRes, &indexR, resTag); /* try to get data from there */ + i++; + } + if(resBundle->fHasFallback) { + // Otherwise, we'll look in parents. + while(*res == RES_BOGUS && dataEntry->fParent != nullptr) { + dataEntry = dataEntry->fParent; + if(dataEntry->fBogus == U_ZERO_ERROR) { + i++; + *res = res_getTableItemByKey(&(dataEntry->fData), dataEntry->fData.rootRes, &indexR, resTag); } } + } - if(*res != RES_BOGUS) { /* If the resource is found in parents, we need to adjust the error */ - if(i>1) { - if(uprv_strcmp(resB->fName, uloc_getDefault())==0 || uprv_strcmp(resB->fName, kRootLocaleName)==0) { - *status = U_USING_DEFAULT_WARNING; - } else { - *status = U_USING_FALLBACK_WARNING; - } - } - *realData = resB; - return (&(resB->fData)); - } else { /* If resource is not found, we need to give an error */ - *status = U_MISSING_RESOURCE_ERROR; - return NULL; + if(*res == RES_BOGUS) { + // If the resource is not found, we need to give an error. + *status = U_MISSING_RESOURCE_ERROR; + return nullptr; + } + // If the resource is found in parents, we need to adjust the error. + if(i>1) { + if(uprv_strcmp(dataEntry->fName, uloc_getDefault())==0 || uprv_strcmp(dataEntry->fName, kRootLocaleName)==0) { + *status = U_USING_DEFAULT_WARNING; + } else { + *status = U_USING_FALLBACK_WARNING; } - } else { - *status = U_MISSING_RESOURCE_ERROR; - return NULL; } + return dataEntry; } static void @@ -972,249 +974,264 @@ ures_close(UResourceBundle* resB) ures_closeBundle(resB, TRUE); } -static UResourceBundle *init_resb_result(const ResourceData *rdata, Resource r, - const char *key, int32_t idx, UResourceDataEntry *realData, - const UResourceBundle *parent, int32_t noAlias, - UResourceBundle *resB, UErrorCode *status) -{ - if(status == NULL || U_FAILURE(*status)) { - return resB; - } - if (parent == NULL) { +namespace { + +UResourceBundle *init_resb_result( + UResourceDataEntry *dataEntry, Resource r, const char *key, int32_t idx, + UResourceDataEntry *validLocaleDataEntry, const char *containerResPath, + int32_t recursionDepth, + UResourceBundle *resB, UErrorCode *status); + +// TODO: Try to refactor further, so that we output a dataEntry + Resource + (optionally) resPath, +// rather than a UResourceBundle. +// May need to entryIncrease() the resulting dataEntry. +UResourceBundle *getAliasTargetAsResourceBundle( + UResourceDataEntry *dataEntry, Resource r, const char *key, int32_t idx, + UResourceDataEntry *validLocaleDataEntry, const char *containerResPath, + int32_t recursionDepth, + UResourceBundle *resB, UErrorCode *status) { + // TODO: When an error occurs: Should we return nullptr vs. resB? + if (U_FAILURE(*status)) { return resB; } + U_ASSERT(RES_GET_TYPE(r) == URES_ALIAS); + int32_t len = 0; + const UChar *alias = res_getAlias(&dataEntry->fData, r, &len); + if(len <= 0) { + // bad alias *status = U_ILLEGAL_ARGUMENT_ERROR; - return NULL; + return resB; } - if(RES_GET_TYPE(r) == URES_ALIAS) { /* This is an alias, need to exchange with real data */ - if(noAlias < URES_MAX_ALIAS_LEVEL) { - int32_t len = 0; - const UChar *alias = res_getAlias(rdata, r, &len); - if(len > 0) { - /* we have an alias, now let's cut it up */ - char stackAlias[200]; - char *chAlias = NULL, *path = NULL, *locale = NULL, *keyPath = NULL; - int32_t capacity; - - /* - * Allocate enough space for both the char * version - * of the alias and parent->fResPath. - * - * We do this so that res_findResource() can modify the path, - * which allows us to remove redundant _res_findResource() variants - * in uresdata.c. - * res_findResource() now NUL-terminates each segment so that table keys - * can always be compared with strcmp() instead of strncmp(). - * Saves code there and simplifies testing and code coverage. - * - * markus 2003oct17 - */ - ++len; /* count the terminating NUL */ - if(parent->fResPath != NULL) { - capacity = (int32_t)uprv_strlen(parent->fResPath) + 1; - } else { - capacity = 0; - } - if(capacity < len) { - capacity = len; + + // Copy the UTF-16 alias string into an invariant-character string. + // + // We do this so that res_findResource() can modify the path, + // which allows us to remove redundant _res_findResource() variants + // in uresdata.c. + // res_findResource() now NUL-terminates each segment so that table keys + // can always be compared with strcmp() instead of strncmp(). + // Saves code there and simplifies testing and code coverage. + // + // markus 2003oct17 + CharString chAlias; + chAlias.appendInvariantChars(alias, len, *status); + if (U_FAILURE(*status)) { + return nullptr; + } + + // We have an alias, now let's cut it up. + const char *path = nullptr, *locale = nullptr, *keyPath = nullptr; + if(chAlias[0] == RES_PATH_SEPARATOR) { + // There is a path included. + char *chAliasData = chAlias.data(); + char *sep = chAliasData + 1; + path = sep; + sep = uprv_strchr(sep, RES_PATH_SEPARATOR); + if(sep != nullptr) { + *sep++ = 0; + } + if(uprv_strcmp(path, "LOCALE") == 0) { + // This is an XPath alias, starting with "/LOCALE/". + // It contains the path to a resource which should be looked up + // starting in the valid locale. + // TODO: Can/should we forbid a /LOCALE alias without key path? + // It seems weird to alias to the same path, just starting from the valid locale. + // That will often yield an infinite loop. + keyPath = sep; + // Read from the valid locale which we already have. + path = locale = nullptr; + } else { + if(uprv_strcmp(path, "ICUDATA") == 0) { /* want ICU data */ + path = nullptr; + } + if (sep == nullptr) { + // TODO: This ends up using the root bundle. Can/should we forbid this? + locale = ""; + } else { + locale = sep; + sep = uprv_strchr(sep, RES_PATH_SEPARATOR); + if(sep != nullptr) { + *sep++ = 0; } - if(capacity <= (int32_t)sizeof(stackAlias)) { - capacity = (int32_t)sizeof(stackAlias); - chAlias = stackAlias; - } else { - chAlias = (char *)uprv_malloc(capacity); - /* test for NULL */ - if(chAlias == NULL) { - *status = U_MEMORY_ALLOCATION_ERROR; - return NULL; - } + keyPath = sep; + } + } + } else { + // No path, start with a locale. + char *sep = chAlias.data(); + locale = sep; + sep = uprv_strchr(sep, RES_PATH_SEPARATOR); + if(sep != nullptr) { + *sep++ = 0; + } + keyPath = sep; + path = dataEntry->fPath; + } + + // Got almost everything, let's try to open. + // First, open the bundle with real data. + LocalUResourceBundlePointer mainRes; + if (locale == nullptr) { + // alias = /LOCALE/keyPath + // Read from the valid locale which we already have. + dataEntry = validLocaleDataEntry; + } else { + UErrorCode intStatus = U_ZERO_ERROR; + // TODO: Shouldn't we use ures_open() for locale data bundles (!noFallback)? + mainRes.adoptInstead(ures_openDirect(path, locale, &intStatus)); + if(U_FAILURE(intStatus)) { + // We failed to open the resource bundle we're aliasing to. + *status = intStatus; + return resB; + } + dataEntry = mainRes->fData; + } + + const char* temp = nullptr; + if(keyPath == nullptr) { + // No key path. This means that we are going to to use the corresponding resource from + // another bundle. + // TODO: Why the special code path? + // Why not put together a key path from containerResPath + key or idx, + // as a comment below suggests, and go into the regular code branch? + // First, we are going to get a corresponding container + // resource to the one we are searching. + r = dataEntry->fData.rootRes; + if(containerResPath) { + chAlias.clear().append(containerResPath, *status); + if (U_FAILURE(*status)) { + return nullptr; + } + char *aKey = chAlias.data(); + // TODO: should res_findResource() return a new dataEntry, too? + r = res_findResource(&dataEntry->fData, r, &aKey, &temp); + } + if(key) { + // We need to make keyPath from the containerResPath and + // current key, if there is a key associated. + chAlias.clear().append(key, *status); + if (U_FAILURE(*status)) { + return nullptr; + } + char *aKey = chAlias.data(); + r = res_findResource(&dataEntry->fData, r, &aKey, &temp); + } else if(idx != -1) { + // If there is no key, but there is an index, try to get by the index. + // Here we have either a table or an array, so get the element. + int32_t type = RES_GET_TYPE(r); + if(URES_IS_TABLE(type)) { + const char *aKey; + r = res_getTableItemByIndex(&dataEntry->fData, r, idx, &aKey); + } else { /* array */ + r = res_getArrayItem(&dataEntry->fData, r, idx); + } + } + if(r != RES_BOGUS) { + resB = init_resb_result( + dataEntry, r, temp, -1, validLocaleDataEntry, nullptr, recursionDepth+1, + resB, status); + } else { + *status = U_MISSING_RESOURCE_ERROR; + } + } else { + // This one is a bit trickier. + // We start finding keys, but after we resolve one alias, the path might continue. + // Consider: + // aliastest:alias { "testtypes/anotheralias/Sequence" } + // anotheralias:alias { "/ICUDATA/sh/CollationElements" } + // aliastest resource should finally have the sequence, not collation elements. + CharString pathBuf(keyPath, *status); + if (U_FAILURE(*status)) { + return nullptr; + } + char *myPath = pathBuf.data(); + containerResPath = nullptr; + // Now we have fallback following here. + for(;;) { + r = dataEntry->fData.rootRes; + // TODO: Move containerResPath = nullptr to here, + // consistent with restarting from the rootRes of another bundle?! + + // This loop handles 'found' resources over several levels. + while(*myPath && U_SUCCESS(*status)) { + r = res_findResource(&(dataEntry->fData), r, &myPath, &temp); + if(r == RES_BOGUS) { + // No resource found, we don't really want to look anymore on this level. + break; } - u_UCharsToChars(alias, chAlias, len); - - if(*chAlias == RES_PATH_SEPARATOR) { - /* there is a path included */ - locale = uprv_strchr(chAlias+1, RES_PATH_SEPARATOR); - if(locale == NULL) { - locale = uprv_strchr(chAlias, 0); /* avoid locale == NULL to make code below work */ - } else { - *locale = 0; - locale++; - } - path = chAlias+1; - if(uprv_strcmp(path, "LOCALE") == 0) { - /* this is an XPath alias, starting with "/LOCALE/" */ - /* it contains the path to a resource which should be looked up */ - /* starting in the requested locale */ - keyPath = locale; - locale = parent->fTopLevelData->fName; /* this is the requested locale's name */ - path = realData->fPath; /* we will be looking in the same package */ - } else { - if(uprv_strcmp(path, "ICUDATA") == 0) { /* want ICU data */ - path = NULL; - } - keyPath = uprv_strchr(locale, RES_PATH_SEPARATOR); - if(keyPath) { - *keyPath = 0; - keyPath++; - } - } - } else { - /* no path, start with a locale */ - locale = chAlias; - keyPath = uprv_strchr(locale, RES_PATH_SEPARATOR); - if(keyPath) { - *keyPath = 0; - keyPath++; - } - path = realData->fPath; + // Found a resource, but it might be an indirection. + resB = init_resb_result( + dataEntry, r, temp, -1, + validLocaleDataEntry, containerResPath, recursionDepth+1, + resB, status); + if (U_FAILURE(*status)) { + break; } - - - { - /* got almost everything, let's try to open */ - /* first, open the bundle with real data */ - UResourceBundle *result = resB; - const char* temp = NULL; - UErrorCode intStatus = U_ZERO_ERROR; - UResourceBundle *mainRes = ures_openDirect(path, locale, &intStatus); - if(U_SUCCESS(intStatus)) { - if(keyPath == NULL) { - /* no key path. This means that we are going to - * to use the corresponding resource from - * another bundle - */ - /* first, we are going to get a corresponding parent - * resource to the one we are searching. - */ - char *aKey = parent->fResPath; - if(aKey) { - uprv_strcpy(chAlias, aKey); /* allocated large enough above */ - aKey = chAlias; - r = res_findResource(&(mainRes->fResData), mainRes->fRes, &aKey, &temp); - } else { - r = mainRes->fRes; - } - if(key) { - /* we need to make keyPath from parent's fResPath and - * current key, if there is a key associated - */ - len = (int32_t)(uprv_strlen(key) + 1); - if(len > capacity) { - capacity = len; - if(chAlias == stackAlias) { - chAlias = (char *)uprv_malloc(capacity); - } else { - chAlias = (char *)uprv_realloc(chAlias, capacity); - } - if(chAlias == NULL) { - ures_close(mainRes); - *status = U_MEMORY_ALLOCATION_ERROR; - return NULL; - } - } - uprv_memcpy(chAlias, key, len); - aKey = chAlias; - r = res_findResource(&(mainRes->fResData), r, &aKey, &temp); - } else if(idx != -1) { - /* if there is no key, but there is an index, try to get by the index */ - /* here we have either a table or an array, so get the element */ - int32_t type = RES_GET_TYPE(r); - if(URES_IS_TABLE(type)) { - r = res_getTableItemByIndex(&(mainRes->fResData), r, idx, (const char **)&aKey); - } else { /* array */ - r = res_getArrayItem(&(mainRes->fResData), r, idx); - } - } - if(r != RES_BOGUS) { - result = init_resb_result(&(mainRes->fResData), r, temp, -1, mainRes->fData, mainRes, noAlias+1, resB, status); - } else { - *status = U_MISSING_RESOURCE_ERROR; - result = resB; - } - } else { - /* this one is a bit trickier. - * we start finding keys, but after we resolve one alias, the path might continue. - * Consider: - * aliastest:alias { "testtypes/anotheralias/Sequence" } - * anotheralias:alias { "/ICUDATA/sh/CollationElements" } - * aliastest resource should finally have the sequence, not collation elements. - */ - UResourceDataEntry *dataEntry = mainRes->fData; - char stackPath[URES_MAX_BUFFER_SIZE]; - char *pathBuf = stackPath, *myPath = pathBuf; - if(uprv_strlen(keyPath) >= UPRV_LENGTHOF(stackPath)) { - pathBuf = (char *)uprv_malloc((uprv_strlen(keyPath)+1)*sizeof(char)); - if(pathBuf == NULL) { - *status = U_MEMORY_ALLOCATION_ERROR; - ures_close(mainRes); - return NULL; - } - } - uprv_strcpy(pathBuf, keyPath); - result = mainRes; - /* now we have fallback following here */ - do { - r = dataEntry->fData.rootRes; - /* this loop handles 'found' resources over several levels */ - while(*myPath && U_SUCCESS(*status)) { - r = res_findResource(&(dataEntry->fData), r, &myPath, &temp); - if(r != RES_BOGUS) { /* found a resource, but it might be an indirection */ - resB = init_resb_result(&(dataEntry->fData), r, temp, -1, dataEntry, result, noAlias+1, resB, status); - if (temp == NULL || uprv_strcmp(keyPath, temp) != 0) { - // the call to init_resb_result() above will set resB->fKeyPath to be - // the same as resB->fKey, throwing away any additional path elements if we - // had them-- if the key path wasn't just a single resource ID, clear out - // the bundle's key path and re-set it to be equal to keyPath - ures_freeResPath(resB); - ures_appendResPath(resB, keyPath, (int32_t)uprv_strlen(keyPath), status); - if(resB->fResPath[resB->fResPathLen-1] != RES_PATH_SEPARATOR) { - ures_appendResPath(resB, RES_PATH_SEPARATOR_S, 1, status); - } - } - result = resB; - if(result) { - r = result->fRes; /* switch to a new resource, possibly a new tree */ - dataEntry = result->fData; - } - } else { /* no resource found, we don't really want to look anymore on this level */ - break; - } - } - dataEntry = dataEntry->fParent; - uprv_strcpy(pathBuf, keyPath); - myPath = pathBuf; - } while(r == RES_BOGUS && dataEntry != NULL); - if(r == RES_BOGUS) { - *status = U_MISSING_RESOURCE_ERROR; - result = resB; - } - if(pathBuf != stackPath) { - uprv_free(pathBuf); - } - } - } else { /* we failed to open the resource we're aliasing to */ - *status = intStatus; - } - if(chAlias != stackAlias) { - uprv_free(chAlias); + if (temp == nullptr || uprv_strcmp(keyPath, temp) != 0) { + // The call to init_resb_result() above will set resB->fKeyPath to be + // the same as resB->fKey, + // throwing away any additional path elements if we had them -- + // if the key path wasn't just a single resource ID, clear out + // the bundle's key path and re-set it to be equal to keyPath. + ures_freeResPath(resB); + ures_appendResPath(resB, keyPath, (int32_t)uprv_strlen(keyPath), status); + if(resB->fResPath[resB->fResPathLen-1] != RES_PATH_SEPARATOR) { + ures_appendResPath(resB, RES_PATH_SEPARATOR_S, 1, status); } - if(mainRes != result) { - ures_close(mainRes); + if (U_FAILURE(*status)) { + break; } - ResourceTracer(resB).maybeTrace("getalias"); - return result; } - } else { - /* bad alias, should be an error */ - *status = U_ILLEGAL_ARGUMENT_ERROR; - return resB; + r = resB->fRes; /* switch to a new resource, possibly a new tree */ + dataEntry = resB->fData; + containerResPath = resB->fResPath; } - } else { + if (U_FAILURE(*status) || r != RES_BOGUS) { + break; + } + // Fall back to the parent bundle, if there is one. + dataEntry = dataEntry->fParent; + if (dataEntry == nullptr) { + *status = U_MISSING_RESOURCE_ERROR; + break; + } + // Copy the same keyPath again. + myPath = pathBuf.data(); + uprv_strcpy(myPath, keyPath); + } + } + if(mainRes.getAlias() == resB) { + mainRes.orphan(); + } + ResourceTracer(resB).maybeTrace("getalias"); + return resB; +} + +// Recursive function, should be called only by itself, by its simpler wrapper, +// or by getAliasTargetAsResourceBundle(). +UResourceBundle *init_resb_result( + UResourceDataEntry *dataEntry, Resource r, const char *key, int32_t idx, + UResourceDataEntry *validLocaleDataEntry, const char *containerResPath, + int32_t recursionDepth, + UResourceBundle *resB, UErrorCode *status) { + // TODO: When an error occurs: Should we return nullptr vs. resB? + if(status == NULL || U_FAILURE(*status)) { + return resB; + } + if (validLocaleDataEntry == nullptr) { + *status = U_ILLEGAL_ARGUMENT_ERROR; + return NULL; + } + if(RES_GET_TYPE(r) == URES_ALIAS) { + // This is an alias, need to exchange with real data. + if(recursionDepth >= URES_MAX_ALIAS_LEVEL) { *status = U_TOO_MANY_ALIASES_ERROR; return resB; } + return getAliasTargetAsResourceBundle( + dataEntry, r, key, idx, + validLocaleDataEntry, containerResPath, recursionDepth, resB, status); } if(resB == NULL) { resB = (UResourceBundle *)uprv_malloc(sizeof(UResourceBundle)); - /* test for NULL */ if (resB == NULL) { *status = U_MEMORY_ALLOCATION_ERROR; return NULL; @@ -1240,20 +1257,20 @@ static UResourceBundle *init_resb_result(const ResourceData *rdata, Resource r, ures_initStackObject(resB); } */ - if(parent != resB) { + if(containerResPath != resB->fResPath) { ures_freeResPath(resB); } } - resB->fData = realData; + resB->fData = dataEntry; entryIncrease(resB->fData); resB->fHasFallback = FALSE; resB->fIsTopLevel = FALSE; resB->fIndex = -1; resB->fKey = key; - /*resB->fParentRes = parent;*/ - resB->fTopLevelData = parent->fTopLevelData; - if(parent->fResPath && parent != resB) { - ures_appendResPath(resB, parent->fResPath, parent->fResPathLen, status); + resB->fValidLocaleDataEntry = validLocaleDataEntry; + if(containerResPath != resB->fResPath) { + ures_appendResPath( + resB, containerResPath, static_cast<int32_t>(uprv_strlen(containerResPath)), status); } if(key != NULL) { ures_appendResPath(resB, key, (int32_t)uprv_strlen(key), status); @@ -1276,13 +1293,23 @@ static UResourceBundle *init_resb_result(const ResourceData *rdata, Resource r, resB->fVersion = NULL; resB->fRes = r; - /*resB->fParent = parent->fRes;*/ - uprv_memmove(&resB->fResData, rdata, sizeof(ResourceData)); - resB->fSize = res_countArrayItems(&(resB->fResData), resB->fRes); + resB->fSize = res_countArrayItems(&resB->getResData(), resB->fRes); ResourceTracer(resB).trace("get"); return resB; } +UResourceBundle *init_resb_result( + UResourceDataEntry *dataEntry, Resource r, const char *key, int32_t idx, + // validLocaleDataEntry + containerResPath + const UResourceBundle *container, + UResourceBundle *resB, UErrorCode *status) { + return init_resb_result( + dataEntry, r, key, idx, + container->fValidLocaleDataEntry, container->fResPath, 0, resB, status); +} + +} // namespace + UResourceBundle *ures_copyResb(UResourceBundle *r, const UResourceBundle *original, UErrorCode *status) { UBool isStackObject; if(U_FAILURE(*status) || r == original) { @@ -1328,7 +1355,7 @@ U_CAPI const UChar* U_EXPORT2 ures_getString(const UResourceBundle* resB, int32_ *status = U_ILLEGAL_ARGUMENT_ERROR; return NULL; } - s = res_getString({resB}, &(resB->fResData), resB->fRes, len); + s = res_getString({resB}, &resB->getResData(), resB->fRes, len); if (s == NULL) { *status = U_RESOURCE_TYPE_MISMATCH; } @@ -1417,7 +1444,7 @@ U_CAPI const uint8_t* U_EXPORT2 ures_getBinary(const UResourceBundle* resB, int3 *status = U_ILLEGAL_ARGUMENT_ERROR; return NULL; } - p = res_getBinary({resB}, &(resB->fResData), resB->fRes, len); + p = res_getBinary({resB}, &resB->getResData(), resB->fRes, len); if (p == NULL) { *status = U_RESOURCE_TYPE_MISMATCH; } @@ -1434,7 +1461,7 @@ U_CAPI const int32_t* U_EXPORT2 ures_getIntVector(const UResourceBundle* resB, i *status = U_ILLEGAL_ARGUMENT_ERROR; return NULL; } - p = res_getIntVector({resB}, &(resB->fResData), resB->fRes, len); + p = res_getIntVector({resB}, &resB->getResData(), resB->fRes, len); if (p == NULL) { *status = U_RESOURCE_TYPE_MISMATCH; } @@ -1512,7 +1539,7 @@ static const UChar* ures_getStringWithAlias(const UResourceBundle *resB, Resourc ures_close(tempRes); return result; } else { - return res_getString({resB, sIndex}, &(resB->fResData), r, len); + return res_getString({resB, sIndex}, &resB->getResData(), r, len); } } @@ -1548,18 +1575,18 @@ U_CAPI const UChar* U_EXPORT2 ures_getNextString(UResourceBundle *resB, int32_t* switch(RES_GET_TYPE(resB->fRes)) { case URES_STRING: case URES_STRING_V2: - return res_getString({resB}, &(resB->fResData), resB->fRes, len); + return res_getString({resB}, &resB->getResData(), resB->fRes, len); case URES_TABLE: case URES_TABLE16: case URES_TABLE32: - r = res_getTableItemByIndex(&(resB->fResData), resB->fRes, resB->fIndex, key); + r = res_getTableItemByIndex(&resB->getResData(), resB->fRes, resB->fIndex, key); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } return ures_getStringWithAlias(resB, r, resB->fIndex, len, status); case URES_ARRAY: case URES_ARRAY16: - r = res_getArrayItem(&(resB->fResData), resB->fRes, resB->fIndex); + r = res_getArrayItem(&resB->getResData(), resB->fRes, resB->fIndex); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } @@ -1608,18 +1635,18 @@ U_CAPI UResourceBundle* U_EXPORT2 ures_getNextResource(UResourceBundle *resB, UR case URES_TABLE: case URES_TABLE16: case URES_TABLE32: - r = res_getTableItemByIndex(&(resB->fResData), resB->fRes, resB->fIndex, &key); + r = res_getTableItemByIndex(&resB->getResData(), resB->fRes, resB->fIndex, &key); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } - return init_resb_result(&(resB->fResData), r, key, resB->fIndex, resB->fData, resB, 0, fillIn, status); + return init_resb_result(resB->fData, r, key, resB->fIndex, resB, fillIn, status); case URES_ARRAY: case URES_ARRAY16: - r = res_getArrayItem(&(resB->fResData), resB->fRes, resB->fIndex); + r = res_getArrayItem(&resB->getResData(), resB->fRes, resB->fIndex); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } - return init_resb_result(&(resB->fResData), r, key, resB->fIndex, resB->fData, resB, 0, fillIn, status); + return init_resb_result(resB->fData, r, key, resB->fIndex, resB, fillIn, status); default: /*return NULL;*/ return fillIn; @@ -1654,18 +1681,18 @@ U_CAPI UResourceBundle* U_EXPORT2 ures_getByIndex(const UResourceBundle *resB, i case URES_TABLE: case URES_TABLE16: case URES_TABLE32: - r = res_getTableItemByIndex(&(resB->fResData), resB->fRes, indexR, &key); + r = res_getTableItemByIndex(&resB->getResData(), resB->fRes, indexR, &key); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } - return init_resb_result(&(resB->fResData), r, key, indexR, resB->fData, resB, 0, fillIn, status); + return init_resb_result(resB->fData, r, key, indexR, resB, fillIn, status); case URES_ARRAY: case URES_ARRAY16: - r = res_getArrayItem(&(resB->fResData), resB->fRes, indexR); + r = res_getArrayItem(&resB->getResData(), resB->fRes, indexR); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } - return init_resb_result(&(resB->fResData), r, key, indexR, resB->fData, resB, 0, fillIn, status); + return init_resb_result(resB->fData, r, key, indexR, resB, fillIn, status); default: /*return NULL;*/ return fillIn; @@ -1693,18 +1720,18 @@ U_CAPI const UChar* U_EXPORT2 ures_getStringByIndex(const UResourceBundle *resB, switch(RES_GET_TYPE(resB->fRes)) { case URES_STRING: case URES_STRING_V2: - return res_getString({resB}, &(resB->fResData), resB->fRes, len); + return res_getString({resB}, &resB->getResData(), resB->fRes, len); case URES_TABLE: case URES_TABLE16: case URES_TABLE32: - r = res_getTableItemByIndex(&(resB->fResData), resB->fRes, indexS, &key); + r = res_getTableItemByIndex(&resB->getResData(), resB->fRes, indexS, &key); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } return ures_getStringWithAlias(resB, r, indexS, len, status); case URES_ARRAY: case URES_ARRAY16: - r = res_getArrayItem(&(resB->fResData), resB->fRes, indexS); + r = res_getArrayItem(&resB->getResData(), resB->fRes, indexS); if(r == RES_BOGUS && resB->fHasFallback) { /* TODO: do the fallback */ } @@ -1812,9 +1839,9 @@ ures_findSubResource(const UResourceBundle *resB, char* path, UResourceBundle *f /* this loop is here because aliasing is resolved on this level, not on res level */ /* so, when we encounter an alias, it is not an aggregate resource, so we return */ do { - res = res_findResource(&(resB->fResData), resB->fRes, &path, &key); + res = res_findResource(&resB->getResData(), resB->fRes, &path, &key); if(res != RES_BOGUS) { - result = init_resb_result(&(resB->fResData), res, key, -1, resB->fData, resB, 0, fillIn, status); + result = init_resb_result(resB->fData, res, key, -1, resB, fillIn, status); resB = result; } else { *status = U_MISSING_RESOURCE_ERROR; @@ -1888,7 +1915,6 @@ ures_getByKeyWithFallback(const UResourceBundle *resB, UResourceBundle *fillIn, UErrorCode *status) { Resource res = RES_BOGUS, rootRes = RES_BOGUS; - /*UResourceDataEntry *realData = NULL;*/ UResourceBundle *helper = NULL; if (status==NULL || U_FAILURE(*status)) { @@ -1901,7 +1927,7 @@ ures_getByKeyWithFallback(const UResourceBundle *resB, int32_t type = RES_GET_TYPE(resB->fRes); if(URES_IS_TABLE(type)) { - res = getTableItemByKeyPath(&(resB->fResData), resB->fRes, inKey); + res = getTableItemByKeyPath(&resB->getResData(), resB->fRes, inKey); const char* key = inKey; if(res == RES_BOGUS) { UResourceDataEntry *dataEntry = resB->fData; @@ -1929,8 +1955,8 @@ ures_getByKeyWithFallback(const UResourceBundle *resB, res = res_findResource(&(dataEntry->fData), rootRes, &myPath, &key); if (RES_GET_TYPE(res) == URES_ALIAS && *myPath) { /* We hit an alias, but we didn't finish following the path. */ - helper = init_resb_result(&(dataEntry->fData), res, NULL, -1, dataEntry, resB, 0, helper, status); - /*helper = init_resb_result(&(dataEntry->fData), res, inKey, -1, dataEntry, resB, 0, helper, status);*/ + helper = init_resb_result(dataEntry, res, NULL, -1, resB, helper, status); + /*helper = init_resb_result(dataEntry, res, inKey, -1, resB, helper, status);*/ if(helper) { dataEntry = helper->fData; rootRes = helper->fRes; @@ -1946,7 +1972,7 @@ ures_getByKeyWithFallback(const UResourceBundle *resB, } while(*myPath); /* Continue until the whole path is consumed */ } } - /*const ResourceData *rd = getFallbackData(resB, &key, &realData, &res, status);*/ + /*dataEntry = getFallbackData(resB, &key, &res, status);*/ if(res != RES_BOGUS) { /* check if resB->fResPath gives the right name here */ if(uprv_strcmp(dataEntry->fName, uloc_getDefault())==0 || uprv_strcmp(dataEntry->fName, kRootLocaleName)==0) { @@ -1955,12 +1981,12 @@ ures_getByKeyWithFallback(const UResourceBundle *resB, *status = U_USING_FALLBACK_WARNING; } - fillIn = init_resb_result(&(dataEntry->fData), res, inKey, -1, dataEntry, resB, 0, fillIn, status); + fillIn = init_resb_result(dataEntry, res, inKey, -1, resB, fillIn, status); } else { *status = U_MISSING_RESOURCE_ERROR; } } else { - fillIn = init_resb_result(&(resB->fResData), res, key, -1, resB->fData, resB, 0, fillIn, status); + fillIn = init_resb_result(resB->fData, res, key, -1, resB, fillIn, status); } } else { @@ -1987,7 +2013,7 @@ void getAllItemsWithFallback( // When the sink sees the no-fallback/no-inheritance marker, // then it would remove the parent's item. // We would deserialize parent values even though they are overridden in a child bundle. - value.setData(&bundle->fResData); + value.setData(bundle->getResData()); UResourceDataEntry *parentEntry = bundle->fData->fParent; UBool hasParent = parentEntry != NULL && U_SUCCESS(parentEntry->fBogus); value.setResource(bundle->fRes, ResourceTracer(bundle)); @@ -2001,35 +2027,30 @@ void getAllItemsWithFallback( // TODO: See if we can refactor ures_getByKeyWithFallback() // and pull out an inner function that takes and returns a UResourceDataEntry // so that we need not create UResourceBundle objects. - UResourceBundle parentBundle; - ures_initStackObject(&parentBundle); - parentBundle.fTopLevelData = parentBundle.fData = parentEntry; - // TODO: What is the difference between bundle fData and fTopLevelData? - uprv_memcpy(&parentBundle.fResData, &parentEntry->fData, sizeof(ResourceData)); - // TODO: Try to replace bundle.fResData with just using bundle.fData->fData. - parentBundle.fHasFallback = !parentBundle.fResData.noFallback; - parentBundle.fIsTopLevel = TRUE; - parentBundle.fRes = parentBundle.fResData.rootRes; - parentBundle.fSize = res_countArrayItems(&(parentBundle.fResData), parentBundle.fRes); - parentBundle.fIndex = -1; + StackUResourceBundle parentBundle; + UResourceBundle &parentRef = parentBundle.ref(); + parentRef.fData = parentEntry; + parentRef.fValidLocaleDataEntry = bundle->fValidLocaleDataEntry; + parentRef.fHasFallback = !parentRef.getResData().noFallback; + parentRef.fIsTopLevel = TRUE; + parentRef.fRes = parentRef.getResData().rootRes; + parentRef.fSize = res_countArrayItems(&parentRef.getResData(), parentRef.fRes); + parentRef.fIndex = -1; entryIncrease(parentEntry); // Look up the container item in the parent bundle. - UResourceBundle containerBundle; - ures_initStackObject(&containerBundle); + StackUResourceBundle containerBundle; const UResourceBundle *rb; UErrorCode pathErrorCode = U_ZERO_ERROR; // Ignore if parents up to root do not have this path. if (bundle->fResPath == NULL || *bundle->fResPath == 0) { - rb = &parentBundle; + rb = parentBundle.getAlias(); } else { - rb = ures_getByKeyWithFallback(&parentBundle, bundle->fResPath, - &containerBundle, &pathErrorCode); + rb = ures_getByKeyWithFallback(parentBundle.getAlias(), bundle->fResPath, + containerBundle.getAlias(), &pathErrorCode); } if (U_SUCCESS(pathErrorCode)) { getAllItemsWithFallback(rb, value, sink, errorCode); } - ures_close(&containerBundle); - ures_close(&parentBundle); } } @@ -2063,7 +2084,7 @@ ures_getValueWithFallback(const UResourceBundle *bundle, const char *path, return; } } - value.setData(&rb->fResData); + value.setData(rb->getResData()); value.setResource(rb->fRes, ResourceTracer(rb)); } @@ -2093,7 +2114,7 @@ ures_getAllItemsWithFallback(const UResourceBundle *bundle, const char *path, U_CAPI UResourceBundle* U_EXPORT2 ures_getByKey(const UResourceBundle *resB, const char* inKey, UResourceBundle *fillIn, UErrorCode *status) { Resource res = RES_BOGUS; - UResourceDataEntry *realData = NULL; + UResourceDataEntry *dataEntry = NULL; const char *key = inKey; if (status==NULL || U_FAILURE(*status)) { @@ -2107,14 +2128,14 @@ U_CAPI UResourceBundle* U_EXPORT2 ures_getByKey(const UResourceBundle *resB, con int32_t type = RES_GET_TYPE(resB->fRes); if(URES_IS_TABLE(type)) { int32_t t; - res = res_getTableItemByKey(&(resB->fResData), resB->fRes, &t, &key); + res = res_getTableItemByKey(&resB->getResData(), resB->fRes, &t, &key); if(res == RES_BOGUS) { key = inKey; if(resB->fHasFallback == TRUE) { - const ResourceData *rd = getFallbackData(resB, &key, &realData, &res, status); + dataEntry = getFallbackData(resB, &key, &res, status); if(U_SUCCESS(*status)) { - /* check if resB->fResPath gives the right name here */ - return init_resb_result(rd, res, key, -1, realData, resB, 0, fillIn, status); + /* check if resB->fResPath gives the right name here */ + return init_resb_result(dataEntry, res, key, -1, resB, fillIn, status); } else { *status = U_MISSING_RESOURCE_ERROR; } @@ -2122,7 +2143,7 @@ U_CAPI UResourceBundle* U_EXPORT2 ures_getByKey(const UResourceBundle *resB, con *status = U_MISSING_RESOURCE_ERROR; } } else { - return init_resb_result(&(resB->fResData), res, key, -1, resB->fData, resB, 0, fillIn, status); + return init_resb_result(resB->fData, res, key, -1, resB, fillIn, status); } } #if 0 @@ -2130,9 +2151,9 @@ U_CAPI UResourceBundle* U_EXPORT2 ures_getByKey(const UResourceBundle *resB, con /* not currently */ else if(RES_GET_TYPE(resB->fRes) == URES_ARRAY && resB->fHasFallback == TRUE) { /* here should go a first attempt to locate the key using index table */ - const ResourceData *rd = getFallbackData(resB, &key, &realData, &res, status); + dataEntry = getFallbackData(resB, &key, &res, status); if(U_SUCCESS(*status)) { - return init_resb_result(rd, res, key, realData, resB, fillIn, status); + return init_resb_result(dataEntry, res, key, resB, fillIn, status); } else { *status = U_MISSING_RESOURCE_ERROR; } @@ -2146,7 +2167,7 @@ U_CAPI UResourceBundle* U_EXPORT2 ures_getByKey(const UResourceBundle *resB, con U_CAPI const UChar* U_EXPORT2 ures_getStringByKey(const UResourceBundle *resB, const char* inKey, int32_t* len, UErrorCode *status) { Resource res = RES_BOGUS; - UResourceDataEntry *realData = NULL; + UResourceDataEntry *dataEntry = NULL; const char* key = inKey; if (status==NULL || U_FAILURE(*status)) { @@ -2161,17 +2182,17 @@ U_CAPI const UChar* U_EXPORT2 ures_getStringByKey(const UResourceBundle *resB, c if(URES_IS_TABLE(type)) { int32_t t=0; - res = res_getTableItemByKey(&(resB->fResData), resB->fRes, &t, &key); + res = res_getTableItemByKey(&resB->getResData(), resB->fRes, &t, &key); if(res == RES_BOGUS) { key = inKey; if(resB->fHasFallback == TRUE) { - const ResourceData *rd = getFallbackData(resB, &key, &realData, &res, status); + dataEntry = getFallbackData(resB, &key, &res, status); if(U_SUCCESS(*status)) { switch (RES_GET_TYPE(res)) { case URES_STRING: case URES_STRING_V2: - return res_getString({resB, key}, rd, res, len); + return res_getString({resB, key}, &dataEntry->fData, res, len); case URES_ALIAS: { const UChar* result = 0; @@ -2193,7 +2214,7 @@ U_CAPI const UChar* U_EXPORT2 ures_getStringByKey(const UResourceBundle *resB, c switch (RES_GET_TYPE(res)) { case URES_STRING: case URES_STRING_V2: - return res_getString({resB, key}, &(resB->fResData), res, len); + return res_getString({resB, key}, &resB->getResData(), res, len); case URES_ALIAS: { const UChar* result = 0; @@ -2212,7 +2233,7 @@ U_CAPI const UChar* U_EXPORT2 ures_getStringByKey(const UResourceBundle *resB, c /* not currently */ else if(RES_GET_TYPE(resB->fRes) == URES_ARRAY && resB->fHasFallback == TRUE) { /* here should go a first attempt to locate the key using index table */ - const ResourceData *rd = getFallbackData(resB, &key, &realData, &res, status); + dataEntry = getFallbackData(resB, &key, &res, status); if(U_SUCCESS(*status)) { // TODO: Tracing return res_getString(rd, res, len); @@ -2281,7 +2302,7 @@ ures_getLocaleByType(const UResourceBundle* resourceBundle, case ULOC_ACTUAL_LOCALE: return resourceBundle->fData->fName; case ULOC_VALID_LOCALE: - return resourceBundle->fTopLevelData->fName; + return resourceBundle->fValidLocaleDataEntry->fName; case ULOC_REQUESTED_LOCALE: default: *status = U_ILLEGAL_ARGUMENT_ERROR; @@ -2352,12 +2373,11 @@ ures_openWithType(UResourceBundle *r, const char* path, const char* localeID, uprv_memset(r, 0, sizeof(UResourceBundle)); ures_setIsStackObject(r, isStackObject); - r->fTopLevelData = r->fData = entry; - uprv_memcpy(&r->fResData, &entry->fData, sizeof(ResourceData)); - r->fHasFallback = openType != URES_OPEN_DIRECT && !r->fResData.noFallback; + r->fValidLocaleDataEntry = r->fData = entry; + r->fHasFallback = openType != URES_OPEN_DIRECT && !r->getResData().noFallback; r->fIsTopLevel = TRUE; - r->fRes = r->fResData.rootRes; - r->fSize = res_countArrayItems(&(r->fResData), r->fRes); + r->fRes = r->getResData().rootRes; + r->fSize = res_countArrayItems(&r->getResData(), r->fRes); r->fIndex = -1; ResourceTracer(r).traceOpen(); @@ -2433,8 +2453,8 @@ ures_countArrayItems(const UResourceBundle* resourceBundle, } ures_getByKey(resourceBundle, resourceKey, &resData, status); - if(resData.fResData.data != NULL) { - int32_t result = res_countArrayItems(&resData.fResData, resData.fRes); + if(resData.getResData().data != NULL) { + int32_t result = res_countArrayItems(&resData.getResData(), resData.fRes); ures_close(&resData); return result; } else { diff --git a/icu4c/source/common/uresdata.h b/icu4c/source/common/uresdata.h index 7c2152e57b5..3596a2282a2 100644 --- a/icu4c/source/common/uresdata.h +++ b/icu4c/source/common/uresdata.h @@ -511,12 +511,13 @@ inline uint32_t res_getUInt(const ResourceTracer& traceInfo, Resource res) { class ResourceDataValue : public ResourceValue { public: ResourceDataValue() : + pResData(nullptr), res(static_cast<Resource>(URES_NONE)), fTraceInfo() {} virtual ~ResourceDataValue(); - void setData(const ResourceData *data) { - resData = *data; + void setData(const ResourceData &data) { + pResData = &data; } void setResource(Resource r, ResourceTracer&& traceInfo) { @@ -524,7 +525,7 @@ public: fTraceInfo = traceInfo; } - const ResourceData &getData() const { return resData; } + const ResourceData &getData() const { return *pResData; } virtual UResType getType() const; virtual const UChar *getString(int32_t &length, UErrorCode &errorCode) const; virtual const UChar *getAliasString(int32_t &length, UErrorCode &errorCode) const; @@ -542,9 +543,7 @@ public: virtual UnicodeString getStringOrFirstOfArray(UErrorCode &errorCode) const; private: - // TODO(ICU-20769): If UResourceBundle.fResData becomes a pointer, - // then remove this value field again and just store a pResData pointer. - ResourceData resData; + const ResourceData *pResData; Resource res; ResourceTracer fTraceInfo; }; diff --git a/icu4c/source/common/uresimp.h b/icu4c/source/common/uresimp.h index 73d37daf9f8..0060a436028 100644 --- a/icu4c/source/common/uresimp.h +++ b/icu4c/source/common/uresimp.h @@ -31,7 +31,6 @@ #define MAGIC2 19641227 #define URES_MAX_ALIAS_LEVEL 256 -#define URES_MAX_BUFFER_SIZE 256 #define EMPTY_SET 0x2205 @@ -61,16 +60,27 @@ struct UResourceDataEntry { #define RES_PATH_SEPARATOR '/' #define RES_PATH_SEPARATOR_S "/" +U_CAPI void U_EXPORT2 ures_initStackObject(UResourceBundle* resB); + +#ifdef __cplusplus + struct UResourceBundle { const char *fKey; /*tag*/ + /** + * The dataEntry for the actual locale in which this item lives. + * Used for accessing the item's data. + * Non-const pointer for reference counting via entryIncrease(). + */ UResourceDataEntry *fData; /*for low-level access*/ char *fVersion; - UResourceDataEntry *fTopLevelData; /* for getting the valid locale */ + /** + * The dataEntry for the valid locale. + * Used for /LOCALE/path alias resolution that starts back from the valid locale, + * rather than from the actual locale of this item which might live in + * an ancestor bundle. + */ + UResourceDataEntry *fValidLocaleDataEntry; char *fResPath; /* full path to the resource: "zh_TW/CollationElements/Sequence" */ - // TODO(ICU-20769): Try to change the by-value fResData into a pointer, - // with the struct in only one place for each bundle. - // Also replace class ResourceDataValue.resData with a pResData pointer again. - ResourceData fResData; char fResBuf[RES_BUFSIZE]; int32_t fResPathLen; Resource fRes; @@ -81,13 +91,9 @@ struct UResourceBundle { int32_t fIndex; int32_t fSize; - /*const UResourceBundle *fParentRes;*/ /* needed to get the actual locale for a child resource */ + inline const ResourceData &getResData() const { return fData->fData; } }; -U_CAPI void U_EXPORT2 ures_initStackObject(UResourceBundle* resB); - -#ifdef __cplusplus - U_NAMESPACE_BEGIN /** @@ -161,9 +167,6 @@ U_CFUNC const char* ures_getPath(const UResourceBundle* resB); */ U_CAPI UBool U_EXPORT2 ures_dumpCacheContents(void); #endif -/*U_CFUNC void ures_appendResPath(UResourceBundle *resB, const char* toAdd, int32_t lenToAdd);*/ -/*U_CFUNC void ures_setResPath(UResourceBundle *resB, const char* toAdd);*/ -/*U_CFUNC void ures_freeResPath(UResourceBundle *resB);*/ /* Candidates for export */ U_CFUNC UResourceBundle *ures_copyResb(UResourceBundle *r, const UResourceBundle *original, UErrorCode *status); diff --git a/icu4c/source/test/cintltst/crestst.c b/icu4c/source/test/cintltst/crestst.c index d79ff8f1e50..b3fd04d5c11 100644 --- a/icu4c/source/test/cintltst/crestst.c +++ b/icu4c/source/test/cintltst/crestst.c @@ -34,7 +34,6 @@ #include "uresimp.h" static void TestOpenDirect(void); -static void TestOpenDirectFillIn(void); static void TestFallback(void); static void TestTable32(void); static void TestFileStream(void); @@ -112,7 +111,6 @@ void addResourceBundleTest(TestNode** root) #if !UCONFIG_NO_FILE_IO && !UCONFIG_NO_LEGACY_CONVERSION addTest(root, &TestConstruction1, "tsutil/crestst/TestConstruction1"); addTest(root, &TestOpenDirect, "tsutil/crestst/TestOpenDirect"); - addTest(root, &TestOpenDirectFillIn, "tsutil/crestst/TestOpenDirectFillIn"); addTest(root, &TestResourceBundles, "tsutil/crestst/TestResourceBundles"); addTest(root, &TestTable32, "tsutil/crestst/TestTable32"); addTest(root, &TestFileStream, "tsutil/crestst/TestFileStream"); @@ -649,47 +647,6 @@ TestOpenDirect(void) { ures_close(defaultLocale); } -static void -TestOpenDirectFillIn(void) { - // Test that ures_openDirectFillIn() opens a stack allocated resource bundle, similar to ures_open(). - // Since ures_openDirectFillIn is just a wrapper function, this is just a very basic test copied from - // the TestOpenDirect test above. - UErrorCode errorCode = U_ZERO_ERROR; - UResourceBundle *item; - UResourceBundle idna_rules; - ures_initStackObject(&idna_rules); - - ures_openDirectFillIn(&idna_rules, loadTestData(&errorCode), "idna_rules", &errorCode); - if(U_FAILURE(errorCode)) { - log_data_err("ures_openDirectFillIn(\"idna_rules\") failed: %s\n", u_errorName(errorCode)); - return; - } - - if(0!=uprv_strcmp("idna_rules", ures_getLocale(&idna_rules, &errorCode))) { - log_err("ures_openDirectFillIn(\"idna_rules\").getLocale()!=idna_rules\n"); - } - errorCode=U_ZERO_ERROR; - - /* try an item in idna_rules, must work */ - item=ures_getByKey(&idna_rules, "UnassignedSet", NULL, &errorCode); - if(U_FAILURE(errorCode)) { - log_err("translit_index.getByKey(local key) failed: %s\n", u_errorName(errorCode)); - errorCode=U_ZERO_ERROR; - } else { - ures_close(item); - } - - /* try an item in root, must fail */ - item=ures_getByKey(&idna_rules, "ShortLanguage", NULL, &errorCode); - if(U_FAILURE(errorCode)) { - errorCode=U_ZERO_ERROR; - } else { - log_err("idna_rules.getByKey(root key) succeeded!\n"); - ures_close(item); - } - ures_close(&idna_rules); -} - static int32_t parseTable32Key(const char *key) { int32_t number; diff --git a/icu4c/source/test/cintltst/creststn.c b/icu4c/source/test/cintltst/creststn.c index 0fcf4d64522..f8c50cb1a82 100644 --- a/icu4c/source/test/cintltst/creststn.c +++ b/icu4c/source/test/cintltst/creststn.c @@ -250,7 +250,6 @@ void addNEWResourceBundleTest(TestNode** root) addTest(root, &TestGetKeywordValues, "tsutil/creststn/TestGetKeywordValues"); addTest(root, &TestGetFunctionalEquivalent,"tsutil/creststn/TestGetFunctionalEquivalent"); addTest(root, &TestJB3763, "tsutil/creststn/TestJB3763"); - addTest(root, &TestStackReuse, "tsutil/creststn/TestStackReuse"); } @@ -2895,23 +2894,6 @@ static void TestFallbackCodes(void) { ures_close(res); } -/* This test will crash if this doesn't work. Results don't need testing. */ -static void TestStackReuse(void) { - UResourceBundle table; - UErrorCode errorCode = U_ZERO_ERROR; - UResourceBundle *rb = ures_open(NULL, "en_US", &errorCode); - - if(U_FAILURE(errorCode)) { - log_data_err("Could not load en_US locale. status=%s\n",myErrorName(errorCode)); - return; - } - ures_initStackObject(&table); - ures_getByKeyWithFallback(rb, "Types", &table, &errorCode); - ures_getByKeyWithFallback(&table, "collation", &table, &errorCode); - ures_close(rb); - ures_close(&table); -} - /* Test ures_getUTF8StringXYZ() --------------------------------------------- */ /* diff --git a/icu4c/source/test/cintltst/creststn.h b/icu4c/source/test/cintltst/creststn.h index 4760d3a5a75..b939a34deec 100644 --- a/icu4c/source/test/cintltst/creststn.h +++ b/icu4c/source/test/cintltst/creststn.h @@ -81,8 +81,6 @@ static void TestJB3763(void); static void TestXPath(void); -static void TestStackReuse(void); - /** * extensive subtests called by TestResourceBundles **/ diff --git a/icu4c/source/test/intltest/restsnew.cpp b/icu4c/source/test/intltest/restsnew.cpp index 724139f0e31..67b9f556326 100644 --- a/icu4c/source/test/intltest/restsnew.cpp +++ b/icu4c/source/test/intltest/restsnew.cpp @@ -48,6 +48,8 @@ void NewResourceBundleTest::runIndexedTest( int32_t index, UBool exec, const cha TESTCASE_AUTO(TestTrace); #endif + TESTCASE_AUTO(TestOpenDirectFillIn); + TESTCASE_AUTO(TestStackReuse); TESTCASE_AUTO_END; } @@ -1550,5 +1552,61 @@ void NewResourceBundleTest::TestTrace() { #endif -//eof +void NewResourceBundleTest::TestOpenDirectFillIn() { + // Test that ures_openDirectFillIn() opens a stack allocated resource bundle, similar to ures_open(). + // Since ures_openDirectFillIn is just a wrapper function, this is just a very basic test copied from + // the crestst.c/TestOpenDirect test. + // ICU-20769: This test was moved to C++ intltest while + // turning UResourceBundle from a C struct into a C++ class. + IcuTestErrorCode errorCode(*this, "TestOpenDirectFillIn"); + UResourceBundle *item; + UResourceBundle idna_rules; + ures_initStackObject(&idna_rules); + + ures_openDirectFillIn(&idna_rules, loadTestData(errorCode), "idna_rules", errorCode); + if(errorCode.errDataIfFailureAndReset("ures_openDirectFillIn(\"idna_rules\") failed\n")) { + return; + } + + if(0!=uprv_strcmp("idna_rules", ures_getLocale(&idna_rules, errorCode))) { + errln("ures_openDirectFillIn(\"idna_rules\").getLocale()!=idna_rules"); + } + errorCode.reset(); + /* try an item in idna_rules, must work */ + item=ures_getByKey(&idna_rules, "UnassignedSet", nullptr, errorCode); + if(errorCode.errDataIfFailureAndReset("translit_index.getByKey(local key) failed\n")) { + // pass + } else { + ures_close(item); + } + + /* try an item in root, must fail */ + item=ures_getByKey(&idna_rules, "ShortLanguage", nullptr, errorCode); + if(errorCode.isFailure()) { + errorCode.reset(); + } else { + errln("idna_rules.getByKey(root key) succeeded but should have failed!"); + ures_close(item); + } + ures_close(&idna_rules); +} + +void NewResourceBundleTest::TestStackReuse() { + // This test will crash if this doesn't work. Results don't need testing. + // ICU-20769: This test was moved to C++ intltest while + // turning UResourceBundle from a C struct into a C++ class. + IcuTestErrorCode errorCode(*this, "TestStackReuse"); + UResourceBundle table; + UResourceBundle *rb = ures_open(nullptr, "en_US", errorCode); + + if(errorCode.errDataIfFailureAndReset("Could not load en_US locale.\n")) { + return; + } + ures_initStackObject(&table); + ures_getByKeyWithFallback(rb, "Types", &table, errorCode); + ures_getByKeyWithFallback(&table, "collation", &table, errorCode); + ures_close(rb); + ures_close(&table); + errorCode.reset(); // ignore U_MISSING_RESOURCE_ERROR etc. +} diff --git a/icu4c/source/test/intltest/restsnew.h b/icu4c/source/test/intltest/restsnew.h index 45cc9309365..565e9887af0 100644 --- a/icu4c/source/test/intltest/restsnew.h +++ b/icu4c/source/test/intltest/restsnew.h @@ -46,6 +46,9 @@ public: void TestTrace(void); #endif + void TestOpenDirectFillIn(); + void TestStackReuse(); + private: /** * The assignment operator has no real implementation. diff --git a/icu4c/source/tools/genrb/derb.cpp b/icu4c/source/tools/genrb/derb.cpp index 997b4001295..70546e1915b 100644 --- a/icu4c/source/tools/genrb/derb.cpp +++ b/icu4c/source/tools/genrb/derb.cpp @@ -386,7 +386,7 @@ static void printHex(UFILE *out, uint8_t what) { static void printOutAlias(UFILE *out, UResourceBundle *parent, Resource r, const char *key, int32_t indent, const char *pname, UErrorCode *status) { static const UChar cr[] = { 0xA }; // LF int32_t len = 0; - const UChar* thestr = res_getAlias(&(parent->fResData), r, &len); + const UChar* thestr = res_getAlias(&(parent->getResData()), r, &len); UChar *string = quotedString(thestr); if(opt_truncate && len > truncsize) { char msg[128]; @@ -594,9 +594,9 @@ static void printOutBundle(UFILE *out, UResourceBundle *resource, int32_t indent for(i = 0; i < resSize; i++) { /* need to know if it's an alias */ if(isTable) { - r = res_getTableItemByIndex(&resource->fResData, resource->fRes, i, &key); + r = res_getTableItemByIndex(&resource->getResData(), resource->fRes, i, &key); } else { - r = res_getArrayItem(&resource->fResData, resource->fRes, i); + r = res_getArrayItem(&resource->getResData(), resource->fRes, i); } if(U_SUCCESS(*status)) { if(res_getPublicType(r) == URES_ALIAS) { -- 2.40.0