commit 5ed09dc9b897a20a362a6918c7ad2714f393acf3
author Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Fri Sep 11 00:03:31 2020 +0200
committer Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Thu Sep 17 01:13:45 2020 +0200
tree 58383e6c7362de427325fa2078cfba3558442a40
parent 7888b23e875cd967e8d33577c28319054cc7ac0f

ICU-20568 Improve MacroProps error handling.

In particular: actually handle Usage memory allocation errors.
Also: correct Scale's error condition.

icu4c/source/i18n/number_skeletons.cpp

diff --git a/icu4c/source/i18n/number_skeletons.cpp b/icu4c/source/i18n/number_skeletons.cpp
index 7bfcb69..5a23bd3 100644
--- a/icu4c/source/i18n/number_skeletons.cpp
+++ b/icu4c/source/i18n/number_skeletons.cpp
@@ -1547,7 +1547,7 @@
 }
 
 bool GeneratorHelpers::usage(const MacroProps& macros, UnicodeString& sb, UErrorCode& /* status */) {
-    if (macros.usage.fLength > 0) {
+    if (macros.usage.isSet()) {
         sb.append(u"usage/", -1);
         sb.append(UnicodeString(macros.usage.fUsage, -1, US_INV));
         return true;

icu4c/source/i18n/number_usageprefs.cpp

diff --git a/icu4c/source/i18n/number_usageprefs.cpp b/icu4c/source/i18n/number_usageprefs.cpp
index 9a1de60..6282e87 100644
--- a/icu4c/source/i18n/number_usageprefs.cpp
+++ b/icu4c/source/i18n/number_usageprefs.cpp
@@ -28,21 +28,33 @@
 using icu::units::ConversionRates;
 
 // Copy constructor
-Usage::Usage(const Usage &other) : fUsage(nullptr), fLength(other.fLength), fError(other.fError) {
-    if (other.fUsage != nullptr) {
-        fUsage = (char *)uprv_malloc(fLength + 1);
-        uprv_strncpy(fUsage, other.fUsage, fLength + 1);
-    }
+Usage::Usage(const Usage &other) : Usage() {
+    this->operator=(other);
 }
 
 // Copy assignment operator
 Usage &Usage::operator=(const Usage &other) {
-    fLength = other.fLength;
-    if (other.fUsage != nullptr) {
-        fUsage = (char *)uprv_malloc(fLength + 1);
-        uprv_strncpy(fUsage, other.fUsage, fLength + 1);
-    }
+    fLength = 0;
     fError = other.fError;
+    if (fUsage != nullptr) {
+        uprv_free(fUsage);
+        fUsage = nullptr;
+    }
+    if (other.fUsage == nullptr) {
+        return *this;
+    }
+    if (U_FAILURE(other.fError)) {
+        // We don't bother trying to allocating memory if we're in any case busy
+        // copying an errored Usage.
+        return *this;
+    }
+    fUsage = (char *)uprv_malloc(other.fLength + 1);
+    if (fUsage == nullptr) {
+        fError = U_MEMORY_ALLOCATION_ERROR;
+        return *this;
+    }
+    fLength = other.fLength;
+    uprv_strncpy(fUsage, other.fUsage, fLength + 1);
     return *this;
 }
 
@@ -82,6 +94,11 @@
     }
     fLength = value.length();
     fUsage = (char *)uprv_malloc(fLength + 1);
+    if (fUsage == nullptr) {
+        fLength = 0;
+        fError = U_MEMORY_ALLOCATION_ERROR;
+        return;
+    }
     uprv_strncpy(fUsage, value.data(), fLength);
     fUsage[fLength] = 0;
 }

icu4c/source/i18n/unicode/numberformatter.h

diff --git a/icu4c/source/i18n/unicode/numberformatter.h b/icu4c/source/i18n/unicode/numberformatter.h
index f84da45..c5aeeca 100644
--- a/icu4c/source/i18n/unicode/numberformatter.h
+++ b/icu4c/source/i18n/unicode/numberformatter.h
@@ -1094,7 +1094,7 @@
     }
 
     UBool copyErrorTo(UErrorCode &status) const {
-        if (fError != U_ZERO_ERROR) {
+        if (U_FAILURE(fError)) {
             status = fError;
             return true;
         }
@@ -1154,13 +1154,15 @@
     int16_t length() const { return fLength; }
 
     /** @internal
-     * Makes a copy of value.
+     * Makes a copy of value. Set to "" to unset.
      */
     void set(StringPiece value);
 
     /** @internal */
     bool isSet() const { return fLength > 0; }
 
+#endif // U_HIDE_INTERNAL_API
+
   private:
     char *fUsage;
     int16_t fLength;
@@ -1168,16 +1170,24 @@
 
     Usage() : fUsage(nullptr), fLength(0), fError(U_ZERO_ERROR) {}
 
+    /** @internal */
+    UBool copyErrorTo(UErrorCode &status) const {
+        if (U_FAILURE(fError)) {
+            status = fError;
+            return true;
+        }
+        return false;
+    }
+
     // Allow NumberFormatterImpl to access fUsage.
     friend class impl::NumberFormatterImpl;
 
     // Allow skeleton generation code to access private members.
     friend class impl::GeneratorHelpers;
 
-    // Allow MacroProps/MicroProps to initialize empty instances.
+    // Allow MacroProps/MicroProps to initialize empty instances and to call
+    // copyErrorTo().
     friend struct impl::MacroProps;
-
-#endif // U_HIDE_INTERNAL_API
 };
 
 // Do not enclose entire SymbolsWrapper with #ifndef U_HIDE_INTERNAL_API, needed for a protected field
@@ -1487,7 +1497,7 @@
     bool copyErrorTo(UErrorCode &status) const {
         return notation.copyErrorTo(status) || precision.copyErrorTo(status) ||
                padder.copyErrorTo(status) || integerWidth.copyErrorTo(status) ||
-               symbols.copyErrorTo(status) || scale.copyErrorTo(status);
+               symbols.copyErrorTo(status) || scale.copyErrorTo(status) || usage.copyErrorTo(status);
     }
 };