ICU-21000 Fix abort called by DateTimePatternGenerator::getDefaultHourCycle

If you call the API getDefaultHourCycle on an empty DateTimePatternGenerator
instance (ie: no locale) then it calls UPRV_UNREACHABLE which calls abort().
We should return an error code instead of aborting.

icu4c/source/i18n/dtptngen.cpp

@@ -713,8 +713,17 @@ void DateTimePatternGenerator::getAllowedHourFormats(const Locale &locale, UErro
 }
 
 UDateFormatHourCycle
-DateTimePatternGenerator::getDefaultHourCycle(UErrorCode& /*status*/) const {
-  switch(fDefaultHourFormatChar) {
+DateTimePatternGenerator::getDefaultHourCycle(UErrorCode& status) const {
+    if (U_FAILURE(status)) {
+        return UDAT_HOUR_CYCLE_23;
+    }
+    if (fDefaultHourFormatChar == 0) {
+        // We need to return something, but the caller should ignore it
+        // anyways since the returned status is a failure.
+        status = U_UNSUPPORTED_ERROR;
+        return UDAT_HOUR_CYCLE_23;
+    }
+    switch (fDefaultHourFormatChar) {
         case CAP_K:
             return UDAT_HOUR_CYCLE_11;
         case LOW_H:

icu4c/source/i18n/unicode/dtptngen.h

@@ -485,9 +485,14 @@ public:
 
 #ifndef U_HIDE_DRAFT_API
     /**
-     * Get the default hour cycle.
-     * @param status  Output param set to success/failure code on exit,
+     * Get the default hour cycle for a locale. Uses the locale that the
+     * DateTimePatternGenerator was initially created with.
+     * 
+     * Cannot be used on an empty DateTimePatternGenerator instance.
+     * 
+     * @param status  Output param set to success/failure code on exit, which
      *                which must not indicate a failure before the function call.
+     *                Set to U_UNSUPPORTED_ERROR if used on an empty instance.
      * @return the default hour cycle.
      * @draft ICU 67
      */

icu4c/source/i18n/unicode/udatpg.h

@@ -654,11 +654,15 @@ udatpg_getPatternForSkeleton(const UDateTimePatternGenerator *dtpg,
 
 #ifndef U_HIDE_DRAFT_API
 /**
- * Return the default hour cycle.
+ * Return the default hour cycle for a locale. Uses the locale that the
+ * UDateTimePatternGenerator was initially created with.
+ * 
+ * Cannot be used on an empty UDateTimePatternGenerator instance.
  * 
  * @param dtpg a pointer to UDateTimePatternGenerator.
  * @param pErrorCode a pointer to the UErrorCode which must not indicate a
- *                   failure before the function call.
+ *                   failure before the function call. Set to U_UNSUPPORTED_ERROR
+ *                   if used on an empty instance.
  * @return the default hour cycle.
  * @draft ICU 67
  */

icu4c/source/test/cintltst/udatpg_test.c

@@ -44,6 +44,7 @@ static void TestBuilder(void);
 static void TestOptions(void);
 static void TestGetFieldDisplayNames(void);
 static void TestGetDefaultHourCycle(void);
+static void TestGetDefaultHourCycleOnEmptyInstance(void);
 
 void addDateTimePatternGeneratorTest(TestNode** root) {
     TESTCASE(TestOpenClose);
@@ -52,6 +53,7 @@ void addDateTimePatternGeneratorTest(TestNode** root) {
     TESTCASE(TestOptions);
     TESTCASE(TestGetFieldDisplayNames);
     TESTCASE(TestGetDefaultHourCycle);
+    TESTCASE(TestGetDefaultHourCycleOnEmptyInstance);
 }
 
 /*
@@ -554,4 +556,28 @@ static void TestGetDefaultHourCycle() {
     }
 }
 
+// Ensure that calling udatpg_getDefaultHourCycle on an empty instance doesn't call UPRV_UNREACHABLE/abort.
+static void TestGetDefaultHourCycleOnEmptyInstance() {
+    UErrorCode status = U_ZERO_ERROR;
+    UDateTimePatternGenerator * dtpgen = udatpg_openEmpty(&status);
+
+    if (U_FAILURE(status)) {
+        log_data_err("ERROR udatpg_openEmpty failed, status: %s \n", myErrorName(status));
+        return;
+    }
+
+    (void)udatpg_getDefaultHourCycle(dtpgen, &status);
+    if (!U_FAILURE(status)) {
+        log_data_err("ERROR expected udatpg_getDefaultHourCycle on an empty instance to fail, status: %s", myErrorName(status));
+    }
+
+    status = U_USELESS_COLLATOR_ERROR;
+    (void)udatpg_getDefaultHourCycle(dtpgen, &status);
+    if (status != U_USELESS_COLLATOR_ERROR) {
+        log_data_err("ERROR udatpg_getDefaultHourCycle shouldn't modify status if it is already failed, status: %s", myErrorName(status));
+    }
+
+    udatpg_close(dtpgen);
+}
+
 #endif

icu4c/source/test/intltest/dtptngts.cpp

@@ -43,6 +43,7 @@ void IntlTestDateTimePatternGeneratorAPI::runIndexedTest( int32_t index, UBool e
         TESTCASE(7, testJjMapping);
         TESTCASE(8, test20640_HourCyclArsEnNH);
         TESTCASE(9, testFallbackWithDefaultRootLocale);
+        TESTCASE(10, testGetDefaultHourCycle_OnEmptyInstance);
         default: name = ""; break;
     }
 }
@@ -1483,4 +1484,27 @@ void IntlTestDateTimePatternGeneratorAPI::testFallbackWithDefaultRootLocale() {
     }
 }
 
+// ICU-21000 Ensure that calling getDefaultHourCycle on an empty instance doesn't call UPRV_UNREACHABLE/abort.
+void IntlTestDateTimePatternGeneratorAPI::testGetDefaultHourCycle_OnEmptyInstance() {
+    UErrorCode status = U_ZERO_ERROR;
+
+    LocalPointer<DateTimePatternGenerator> dtpg(DateTimePatternGenerator::createEmptyInstance(status), status);
+    if (U_FAILURE(status)) {
+        errln("ERROR: createEmptyInstance failed, status: %s", u_errorName(status));
+        return;
+    }
+    (void)dtpg->getDefaultHourCycle(status);
+    if (!U_FAILURE(status)) {
+        errln("ERROR: expected getDefaultHourCycle on an empty instance to fail, status: %s", u_errorName(status));
+        return;
+    }
+
+    status = U_USELESS_COLLATOR_ERROR;
+    (void)dtpg->getDefaultHourCycle(status);
+    if (status != U_USELESS_COLLATOR_ERROR) {
+        errln("ERROR: getDefaultHourCycle shouldn't modify status if it is already failed, status: %s", u_errorName(status));
+        return;
+    }
+}
+
 #endif /* #if !UCONFIG_NO_FORMATTING */

icu4c/source/test/intltest/dtptngts.h

@@ -35,6 +35,7 @@ private:
     void testJjMapping();
     void test20640_HourCyclArsEnNH();
     void testFallbackWithDefaultRootLocale();
+    void testGetDefaultHourCycle_OnEmptyInstance();
 };
 
 #endif /* #if !UCONFIG_NO_FORMATTING */