Removing fCurrency to always use fId

diff --git a/icu4c/source/i18n/measunit.cpp b/icu4c/source/i18n/measunit.cpp
index 0886b23caf75e85e0ca32dbf5e9bef532365233a..43811a5331dc47e403ec8eee4f00ad7c133632fd 100644 (file)
--- a/icu4c/source/i18n/measunit.cpp
+++ b/icu4c/source/i18n/measunit.cpp
@@ -2011,10 +2011,10 @@ MeasureUnit::MeasureUnit() : MeasureUnit(kBaseTypeIdx, kBaseSubTypeIdx) {
 
 MeasureUnit::MeasureUnit(int32_t typeId, int32_t subTypeId)
         : fId(nullptr), fSubTypeId(subTypeId), fTypeId(typeId) {
-    fCurrency[0] = 0;
 }
 
-MeasureUnit::MeasureUnit(const MeasureUnit &other) {
+MeasureUnit::MeasureUnit(const MeasureUnit &other)
+        : fId(nullptr) {
     *this = other;
 }
 
@@ -2022,7 +2022,6 @@ MeasureUnit::MeasureUnit(MeasureUnit &&other) noexcept
         : fId(other.fId),
         fSubTypeId(other.fSubTypeId),
         fTypeId(other.fTypeId) {
-    uprv_strcpy(fCurrency, other.fCurrency);
     other.fId = nullptr;
 }
 
@@ -2030,21 +2029,19 @@ MeasureUnit &MeasureUnit::operator=(const MeasureUnit &other) {
     if (this == &other) {
         return *this;
     }
+    uprv_free(fId);
     if (other.fId) {
-        auto* id = static_cast<char*>(uprv_malloc(uprv_strlen(other.fId) + 1));
-        if (!id) {
+        fId = uprv_strdup(other.fId);
+        if (!fId) {
             // Unrecoverable allocation error; set to the default unit
             *this = MeasureUnit();
             return *this;
         }
-        uprv_strcpy(id, other.fId);
-        fId = id;
     } else {
         fId = nullptr;
     }
     fTypeId = other.fTypeId;
     fSubTypeId = other.fSubTypeId;
-    uprv_strcpy(fCurrency, other.fCurrency);
     return *this;
 }
 
@@ -2052,11 +2049,11 @@ MeasureUnit &MeasureUnit::operator=(MeasureUnit &&other) noexcept {
     if (this == &other) {
         return *this;
     }
+    uprv_free(fId);
     fId = other.fId;
     other.fId = nullptr;
     fTypeId = other.fTypeId;
     fSubTypeId = other.fSubTypeId;
-    uprv_strcpy(fCurrency, other.fCurrency);
     return *this;
 }
 
@@ -2065,20 +2062,28 @@ MeasureUnit *MeasureUnit::clone() const {
 }
 
 MeasureUnit::~MeasureUnit() {
-    uprv_free(const_cast<char*>(fId));
+    uprv_free(fId);
     fId = nullptr;
 }
 
 const char *MeasureUnit::getType() const {
+    // We have a type & subtype only if fTypeId is present.
+    if (fTypeId == -1) {
+        return "";
+    }
     return gTypes[fTypeId];
 }
 
 const char *MeasureUnit::getSubtype() const {
-    return fCurrency[0] == 0 ? gSubTypes[getOffset()] : fCurrency;
+    // We have a type & subtype only if fTypeId is present.
+    if (fTypeId == -1) {
+        return "";
+    }
+    return toString();
 }
 
 const char *MeasureUnit::toString() const {
-    return fId ? fId : getSubtype();
+    return fId ? fId : gSubTypes[getOffset()];
 }
 
 UBool MeasureUnit::operator==(const UObject& other) const {
@@ -2225,6 +2230,10 @@ MeasureUnit MeasureUnit::resolveUnitPerUnit(
         const MeasureUnit &unit, const MeasureUnit &perUnit, bool* isResolved) {
     int32_t unitOffset = unit.getOffset();
     int32_t perUnitOffset = perUnit.getOffset();
+    if (unitOffset == -1 || perUnitOffset == -1) {
+        *isResolved = false;
+        return MeasureUnit();
+    }
 
     // binary search for (unitOffset, perUnitOffset)
     int32_t start = 0;
@@ -2278,12 +2287,18 @@ void MeasureUnit::initCurrency(const char *isoCurrency) {
     fTypeId = result;
     result = binarySearch(
             gSubTypes, gOffsets[fTypeId], gOffsets[fTypeId + 1], isoCurrency);
-    if (result != -1) {
-        fSubTypeId = result - gOffsets[fTypeId];
-    } else {
-        uprv_strncpy(fCurrency, isoCurrency, UPRV_LENGTHOF(fCurrency));
-        fCurrency[3] = 0;
+    if (result == -1) {
+        fId = uprv_strdup(isoCurrency);
+        if (fId) {
+            fSubTypeId = -1;
+            return;
+        }
+        // malloc error: fall back to the undefined currency
+        result = binarySearch(
+            gSubTypes, gOffsets[fTypeId], gOffsets[fTypeId + 1], "XXX");
+        U_ASSERT(result != -1);
     }
+    fSubTypeId = result - gOffsets[fTypeId];
 }
 
 void MeasureUnit::initNoUnit(const char *subtype) {
@@ -2298,10 +2313,14 @@ void MeasureUnit::initNoUnit(const char *subtype) {
 void MeasureUnit::setTo(int32_t typeId, int32_t subTypeId) {
     fTypeId = typeId;
     fSubTypeId = subTypeId;
-    fCurrency[0] = 0;
+    uprv_free(fId);
+    fId = nullptr;
 }
 
 int32_t MeasureUnit::getOffset() const {
+    if (fTypeId < 0 || fSubTypeId < 0) {
+        return -1;
+    }
     return gOffsets[fTypeId] + fSubTypeId;
 }
 

diff --git a/icu4c/source/i18n/unicode/measunit.h b/icu4c/source/i18n/unicode/measunit.h
index cca944cc8ff888c81d51bd17544b5193c7785528..16790f8a3d9d792bc0abb9c7ac9478b045d2fbaa 100644 (file)
--- a/icu4c/source/i18n/unicode/measunit.h
+++ b/icu4c/source/i18n/unicode/measunit.h
@@ -3697,8 +3697,10 @@ class U_I18N_API MeasureUnit: public UObject {
 
 private:
 
-    const char* fId;
-    char fCurrency[4];
+    // If non-null, fId is owned by the MeasureUnit.
+    char* fId;
+
+    // These two ints are indices into static string lists in measunit.cpp
     int16_t fSubTypeId;
     int8_t fTypeId;
 

diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp
index 8b0f7e0a77b86a2d3d31650e2307eed138b87cf0..b1b032a05a3057c4c0b122386a3dcdc20a24a69c 100644 (file)
--- a/icu4c/source/test/intltest/numfmtst.cpp
+++ b/icu4c/source/test/intltest/numfmtst.cpp
@@ -2164,6 +2164,8 @@ void NumberFormatTest::TestCurrencyUnit(void){
     static const UChar BAD2[] = u"??A";
     static const UChar XXX[]  = u"XXX";
     static const char XXX8[]  =  "XXX";
+    static const UChar XYZ[]  = u"XYZ";
+    static const char XYZ8[]  =  "XYZ";
     static const UChar INV[]  = u"{$%";
     static const char INV8[]  =  "{$%";
     static const UChar ZZZ[]  = u"zz";
@@ -2180,10 +2182,16 @@ void NumberFormatTest::TestCurrencyUnit(void){
 
     CurrencyUnit cu(USD, ec);
     assertSuccess("CurrencyUnit", ec);
-
     assertEquals("getISOCurrency()", USD, cu.getISOCurrency());
     assertEquals("getSubtype()", USD8, cu.getSubtype());
 
+    // Test XYZ, a valid but non-standard currency.
+    // Note: Country code XY is private-use, so XYZ should remain unallocated.
+    CurrencyUnit extended(XYZ, ec);
+    assertSuccess("non-standard", ec);
+    assertEquals("non-standard", XYZ, extended.getISOCurrency());
+    assertEquals("non-standard", XYZ8, extended.getSubtype());
+
     CurrencyUnit inv(INV, ec);
     assertEquals("non-invariant", U_INVARIANT_CONVERSION_ERROR, ec);
     assertEquals("non-invariant", XXX, inv.getISOCurrency());
@@ -2257,15 +2265,20 @@ void NumberFormatTest::TestCurrencyUnit(void){
     // Test slicing
     MeasureUnit sliced1 = cu;
     MeasureUnit sliced2 = cu;
+    MeasureUnit sliced3 = extended;
     assertEquals("Subtype after slicing 1", USD8, sliced1.getSubtype());
     assertEquals("Subtype after slicing 2", USD8, sliced2.getSubtype());
+    assertEquals("Subtype after slicing 3", XYZ8, sliced3.getSubtype());
     CurrencyUnit restored1(sliced1, ec);
     CurrencyUnit restored2(sliced2, ec);
+    CurrencyUnit restored3(sliced3, ec);
     assertSuccess("Restoring from MeasureUnit", ec);
     assertEquals("Subtype after restoring 1", USD8, restored1.getSubtype());
     assertEquals("Subtype after restoring 2", USD8, restored2.getSubtype());
+    assertEquals("Subtype after restoring 3", XYZ8, restored3.getSubtype());
     assertEquals("ISO Code after restoring 1", USD, restored1.getISOCurrency());
     assertEquals("ISO Code after restoring 2", USD, restored2.getISOCurrency());
+    assertEquals("ISO Code after restoring 3", XYZ, restored3.getISOCurrency());
 
     // Test copy constructor failure
     LocalPointer<MeasureUnit> meter(MeasureUnit::createMeter(ec));