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