After Markus’s review

Author: eggrobin

icu4c/source/common/uniset_props.cpp

@@ -147,26 +147,28 @@ isPOSIXClose(const UnicodeString &pattern, int32_t pos) {
 // Strips leading and trailing spaces and turns runs of spaces into single spaces.
 // This should be replaced by UAX44-LM1 and UAX44-LM2 skeletonizations as part of ICU-3736.
 template<typename CharT>
-UBool mungeCharName(char* dst, std::basic_string_view<CharT> src, int32_t dstCapacity) {
+UBool mungeCharName(std::basic_string_view<CharT> src, char* dst, int32_t dstCapacity) {
     int32_t j = 0;
     --dstCapacity; /* make room for term. zero */
     if constexpr (!std::is_same_v<CharT, char>) {
         if (!uprv_isInvariantUString(src.data(), static_cast<int32_t>(src.size()))) {
             return false;
         }
     }
-    for (CharT ch : src) {
-        if (ch == ' ' && (j == 0 || (j > 0 && dst[j - 1] == ' '))) {
-            continue;
-        }
-        if (j >= dstCapacity) return false;
+    for (CharT uch : src) {
+        char ch;
         if constexpr (std::is_same_v<CharT, char>) {
-            dst[j++] = ch;
+            ch = uch;
         } else {
             // This would want to be UCHAR_TO_CHAR but that is defined in uinvchar.cpp. This function
             // should not last long anyway (famous last words)…
-            u_UCharsToChars(&ch, &dst[j++], 1);
+            u_UCharsToChars(&uch, &ch, 1);
         }
+        if (ch == ' ' && (j == 0 || (j > 0 && dst[j - 1] == ' '))) {
+            continue;
+        }
+        if (j >= dstCapacity) return false;
+        dst[j++] = ch;
     }
     if (j > 0 && dst[j-1] == ' ') --j;
     dst[j] = 0;
@@ -179,8 +181,8 @@ template<typename CharT>
 UChar32 getCharacterByName(const std::basic_string_view<CharT> name) {
     // Must munge name, since u_charFromName() does not do 'loose' matching.
     char buf[128]; // it suffices that this be > uprv_getMaxCharNameLength
-    if (!mungeCharName<CharT>(buf, name, sizeof(buf))) {
-      return U_SENTINEL;
+    if (!mungeCharName(name, buf, sizeof(buf))) {
+        return U_SENTINEL;
     }
     for (const UCharNameChoice nameChoice : std::array{U_EXTENDED_CHAR_NAME, U_CHAR_NAME_ALIAS}) {
         UErrorCode ec = U_ZERO_ERROR;
@@ -702,10 +704,10 @@ class UnicodeSet::Lexer {
                     }
                     start = parsePosition_.getIndex();
                 } else if (last == u'}') {
-                    const UChar32 result = getCharacterByName(
-                    std::u16string_view(pattern_).substr(start, parsePosition_.getIndex() - 1 - start));
-                    if (result == U_SENTINEL || (hex.has_value() && result != hex) ||
-                        (literal.has_value() && result != literal)) {
+                    const UChar32 result = getCharacterByName(std::u16string_view(pattern_).substr(
+                        start, parsePosition_.getIndex() - 1 - start));
+                    if (result < 0 || (hex.has_value() && result != hex) ||
+                            (literal.has_value() && result != literal)) {
                         errorCode = U_ILLEGAL_ARGUMENT_ERROR;
                         return {};
                     }
@@ -1475,8 +1477,9 @@ UnicodeSet::applyPropertyAlias(const UnicodeString& prop,
                 }
             case UCHAR_NAME:
                 {
-                    const UChar32 ch = getCharacterByName<char>(vname.data());
-                    if (ch == U_SENTINEL) {
+                    const UChar32 ch =
+                        getCharacterByName<char>(std::string_view(vname.data(), vname.length()));
+                    if (ch < 0) {
                       FAIL(ec);
                     }
                     add(ch);
@@ -1490,7 +1493,9 @@ UnicodeSet::applyPropertyAlias(const UnicodeString& prop,
                     // Must munge name, since u_versionFromString() does not do
                     // 'loose' matching.
                     char buf[128];
-                    if (!mungeCharName<char>(buf, vname.data(), sizeof(buf))) FAIL(ec);
+                    if (!mungeCharName(std::string_view(vname.data(), vname.length()), buf,
+                                       sizeof(buf)))
+                        FAIL(ec);
                     UVersionInfo version;
                     u_versionFromString(version, buf);
                     applyFilter(versionFilter, &version,