ICU-20414 Add internal ures_openDirectFillIn API, use in getTZDataVersion to avoid memory allocation for UResourceBundle.

icu4c/source/common/unicode/urename.h

@@ -1552,6 +1552,7 @@
 #define ures_open U_ICU_ENTRY_POINT_RENAME(ures_open)
 #define ures_openAvailableLocales U_ICU_ENTRY_POINT_RENAME(ures_openAvailableLocales)
 #define ures_openDirect U_ICU_ENTRY_POINT_RENAME(ures_openDirect)
+#define ures_openDirectFillIn U_ICU_ENTRY_POINT_RENAME(ures_openDirectFillIn)
 #define ures_openFillIn U_ICU_ENTRY_POINT_RENAME(ures_openFillIn)
 #define ures_openNoDefault U_ICU_ENTRY_POINT_RENAME(ures_openNoDefault)
 #define ures_openU U_ICU_ENTRY_POINT_RENAME(ures_openU)

icu4c/source/common/unicode/ures.h

@@ -333,19 +333,19 @@ ures_getLocaleByType(const UResourceBundle* resourceBundle,
 
 #ifndef U_HIDE_INTERNAL_API
 /**
- * Same as ures_open() but uses the fill-in parameter instead of allocating
- * a bundle, if r!=NULL.
+ * Same as ures_open() but uses the fill-in parameter instead of allocating a new bundle.
+ *
  * TODO need to revisit usefulness of this function
  *      and usage model for fillIn parameters without knowing sizeof(UResourceBundle)
- * @param r The resourcebundle to open
+ * @param r The existing UResourceBundle to fill in. If NULL then status will be
+ *               set to U_ILLEGAL_ARGUMENT_ERROR.
  * @param packageName   The packageName and locale together point to an ICU udata object,
  *                      as defined by <code> udata_open( packageName, "res", locale, err) </code>
  *                      or equivalent.  Typically, packageName will refer to a (.dat) file, or to
  *                      a package registered with udata_setAppData(). Using a full file or directory
  *                      pathname for packageName is deprecated. If NULL, ICU data will be used.
  * @param localeID specifies the locale for which we want to open the resource
- * @param status The error code
- * @return a newly allocated resource bundle or NULL if it doesn't exist.
+ * @param status The error code.
  * @internal
  */
 U_INTERNAL void U_EXPORT2

icu4c/source/common/uresbund.cpp

@@ -2313,11 +2313,13 @@ ures_openDirect(const char* path, const char* localeID, UErrorCode* status) {
 }
 
 /**
- *  API: This function is used to open a resource bundle 
+ *  Internal API: This function is used to open a resource bundle 
  *  proper fallback chaining is executed while initialization. 
  *  The result is stored in cache for later fallback search.
+ * 
+ * Same as ures_open(), but uses the fill-in parameter and does not allocate a new bundle.
  */
-U_CAPI void U_EXPORT2
+U_INTERNAL void U_EXPORT2
 ures_openFillIn(UResourceBundle *r, const char* path,
                 const char* localeID, UErrorCode* status) {
     if(U_SUCCESS(*status) && r == NULL) {
@@ -2327,6 +2329,18 @@ ures_openFillIn(UResourceBundle *r, const char* path,
     ures_openWithType(r, path, localeID, URES_OPEN_LOCALE_DEFAULT_ROOT, status);
 }
 
+/**
+ * Same as ures_openDirect(), but uses the fill-in parameter and does not allocate a new bundle.
+ */
+U_INTERNAL void U_EXPORT2
+ures_openDirectFillIn(UResourceBundle *r, const char* path, const char* localeID, UErrorCode* status) {
+    if(U_SUCCESS(*status) && r == NULL) {
+        *status = U_ILLEGAL_ARGUMENT_ERROR;
+        return;
+    }
+    ures_openWithType(r, path, localeID, URES_OPEN_DIRECT, status);
+}
+
 /**
  *  API: Counts members. For arrays and tables, returns number of resources.
  *  For strings, returns 1.

icu4c/source/common/uresimp.h

@@ -327,4 +327,27 @@ U_CAPI const char* U_EXPORT2
 ures_getLocaleInternal(const UResourceBundle* resourceBundle, 
                UErrorCode* status);
 
+/**
+ * Same as ures_openDirect() but uses the fill-in parameter instead of allocating a new bundle.
+ * 
+ * @param r The existing UResourceBundle to fill in. If NULL then status will be
+ *          set to U_ILLEGAL_ARGUMENT_ERROR.
+ * @param packageName   The packageName and locale together point to an ICU udata object,
+ *                      as defined by <code> udata_open( packageName, "res", locale, err) </code>
+ *                      or equivalent.  Typically, packageName will refer to a (.dat) file, or to
+ *                      a package registered with udata_setAppData(). Using a full file or directory
+ *                      pathname for packageName is deprecated. If NULL, ICU data will be used.
+ * @param locale  specifies the locale for which we want to open the resource
+ *                if NULL, the default locale will be used. If strlen(locale) == 0
+ *                root locale will be used.
+ * @param status The error code.
+ * @see ures_openDirect
+ * @internal
+ */
+U_CAPI void U_EXPORT2
+ures_openDirectFillIn(UResourceBundle *r,
+                      const char *packageName,
+                      const char *locale,
+                      UErrorCode *status);
+
 #endif /*URESIMP_H*/

icu4c/source/i18n/timezone.cpp

@@ -1497,8 +1497,10 @@ TimeZone::hasSameRules(const TimeZone& other) const
 static void U_CALLCONV initTZDataVersion(UErrorCode &status) {
     ucln_i18n_registerCleanup(UCLN_I18N_TIMEZONE, timeZone_cleanup);
     int32_t len = 0;
-    UResourceBundle *bundle = ures_openDirect(NULL, kZONEINFO, &status);
-    const UChar *tzver = ures_getStringByKey(bundle, kTZVERSION, &len, &status);
+    UResourceBundle bundle;
+    ures_initStackObject(&bundle);
+    ures_openDirectFillIn(&bundle, NULL, kZONEINFO, &status);
+    const UChar *tzver = ures_getStringByKey(&bundle, kTZVERSION, &len, &status);
 
     if (U_SUCCESS(status)) {
         if (len >= (int32_t)sizeof(TZDATA_VERSION)) {
@@ -1507,8 +1509,7 @@ static void U_CALLCONV initTZDataVersion(UErrorCode &status) {
         }
         u_UCharsToChars(tzver, TZDATA_VERSION, len);
     }
-    ures_close(bundle);
-
+    ures_close(&bundle);
 }
 
 const char*

icu4c/source/test/cintltst/crestst.c

@@ -31,8 +31,10 @@
 #include "unicode/ures.h"
 #include "crestst.h"
 #include "unicode/ctest.h"
+#include "uresimp.h"
 
 static void TestOpenDirect(void);
+static void TestOpenDirectFillIn(void);
 static void TestFallback(void);
 static void TestTable32(void);
 static void TestFileStream(void);
@@ -96,6 +98,7 @@ 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");
@@ -614,6 +617,47 @@ TestOpenDirect(void) {
     ures_close(te_IN);
 }
 
+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;

icu4c/source/test/cintltst/creststn.c

@@ -1178,7 +1178,7 @@ static void TestErrorConditions(){
         log_err("ERROR: ures_openU() is supposed to fail path =%s with status != U_ZERO_ERROR\n", austrdup(utestdatapath));
         ures_close(teRes);
     }
-    /*Test ures_openFillIn with UResourceBundle = NULL*/
+    /*Test ures_openFillIn fails when input UResourceBundle parameter is NULL*/
     log_verbose("Testing ures_openFillIn with UResourceBundle = NULL.....\n");
     status=U_ZERO_ERROR;
     ures_openFillIn(NULL, testdatapath, "te", &status);
@@ -1186,6 +1186,14 @@ static void TestErrorConditions(){
         log_err("ERROR: ures_openFillIn with UResourceBundle= NULL should fail.  Expected U_ILLEGAL_ARGUMENT_ERROR, Got: %s\n",
                         myErrorName(status));
     }
+    /*Test ures_openDirectFillIn fails when input UResourceBundle parameter is NULL*/
+    log_verbose("Testing ures_openDirectFillIn with UResourceBundle = NULL.....\n");
+    status=U_ZERO_ERROR;
+    ures_openDirectFillIn(NULL, testdatapath, "te", &status);
+    if(status != U_ILLEGAL_ARGUMENT_ERROR){
+        log_err("ERROR: ures_openDirectFillIn with UResourceBundle= NULL should fail.  Expected U_ILLEGAL_ARGUMENT_ERROR, Got: %s\n",
+                        myErrorName(status));
+    }
     /*Test ures_getLocale() with status != U_ZERO_ERROR*/
     status=U_ZERO_ERROR;
     teRes=ures_openU(utestdatapath, "te", &status);