From 93d9dc3fe27551466a83412ea52b6ef08adce9f4 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Wed, 27 Sep 2017 03:52:08 +0000 Subject: [PATCH] ICU-13177 Cleaning up Valgrind errors in NumberFormatter unit test suite. X-SVN-Rev: 40470 --- icu4c/source/i18n/number_affixutils.cpp | 7 +++- icu4c/source/i18n/number_stringbuilder.cpp | 40 ++++++++++++++----- .../intltest/numbertest_decimalquantity.cpp | 2 +- .../intltest/numbertest_patternmodifier.cpp | 2 +- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/icu4c/source/i18n/number_affixutils.cpp b/icu4c/source/i18n/number_affixutils.cpp index ec8b5b05240..a412dcc8b7f 100644 --- a/icu4c/source/i18n/number_affixutils.cpp +++ b/icu4c/source/i18n/number_affixutils.cpp @@ -152,6 +152,7 @@ AffixUtils::unescape(const CharSequence &affixPattern, NumberStringBuilder &outp AffixTag tag; while (hasNext(tag, affixPattern)) { tag = nextToken(tag, affixPattern, status); + if (U_FAILURE(status)) { return length; } if (tag.type == TYPE_CURRENCY_OVERFLOW) { // Don't go to the provider for this special case length += output.insertCodePoint(position + length, 0xFFFD, UNUM_CURRENCY_FIELD, status); @@ -171,6 +172,7 @@ int32_t AffixUtils::unescapedCodePointCount(const CharSequence &affixPattern, AffixTag tag; while (hasNext(tag, affixPattern)) { tag = nextToken(tag, affixPattern, status); + if (U_FAILURE(status)) { return length; } if (tag.type == TYPE_CURRENCY_OVERFLOW) { length += 1; } else if (tag.type < 0) { @@ -190,6 +192,7 @@ AffixUtils::containsType(const CharSequence &affixPattern, AffixPatternType type AffixTag tag; while (hasNext(tag, affixPattern)) { tag = nextToken(tag, affixPattern, status); + if (U_FAILURE(status)) { return false; } if (tag.type == type) { return true; } @@ -204,6 +207,7 @@ bool AffixUtils::hasCurrencySymbols(const CharSequence &affixPattern, UErrorCode AffixTag tag; while (hasNext(tag, affixPattern)) { tag = nextToken(tag, affixPattern, status); + if (U_FAILURE(status)) { return false; } if (tag.type < 0 && getFieldForType(tag.type) == UNUM_CURRENCY_FIELD) { return true; } @@ -220,6 +224,7 @@ UnicodeString AffixUtils::replaceType(const CharSequence &affixPattern, AffixPat AffixTag tag; while (hasNext(tag, affixPattern)) { tag = nextToken(tag, affixPattern, status); + if (U_FAILURE(status)) { return output; } if (tag.type == type) { output.replace(tag.offset - 1, 1, replacementChar); } @@ -371,7 +376,7 @@ AffixTag AffixUtils::nextToken(AffixTag tag, const CharSequence &patternString, } bool AffixUtils::hasNext(const AffixTag &tag, const CharSequence &string) { - // First check for the {-1} and {0} initializer syntax. + // First check for the {-1} and default initializer syntax. if (tag.offset < 0) { return false; } else if (tag.offset == 0) { diff --git a/icu4c/source/i18n/number_stringbuilder.cpp b/icu4c/source/i18n/number_stringbuilder.cpp index 1703c97b6df..32be6a59002 100644 --- a/icu4c/source/i18n/number_stringbuilder.cpp +++ b/icu4c/source/i18n/number_stringbuilder.cpp @@ -7,6 +7,26 @@ using namespace icu::number::impl; +namespace { + +// A version of uprv_memcpy that checks for length 0. +// By default, uprv_memcpy requires a length of at least 1. +inline void uprv_memcpy2(void* dest, const void* src, size_t len) { + if (len > 0) { + uprv_memcpy(dest, src, len); + } +} + +// A version of uprv_memmove that checks for length 0. +// By default, uprv_memmove requires a length of at least 1. +inline void uprv_memmove2(void* dest, const void* src, size_t len) { + if (len > 0) { + uprv_memmove(dest, src, len); + } +} + +} // namespace + NumberStringBuilder::NumberStringBuilder() = default; NumberStringBuilder::~NumberStringBuilder() { @@ -54,8 +74,8 @@ NumberStringBuilder &NumberStringBuilder::operator=(const NumberStringBuilder &o fFields.heap.ptr = newFields; } - uprv_memcpy(getCharPtr(), other.getCharPtr(), sizeof(char16_t) * capacity); - uprv_memcpy(getFieldPtr(), other.getFieldPtr(), sizeof(Field) * capacity); + uprv_memcpy2(getCharPtr(), other.getCharPtr(), sizeof(char16_t) * capacity); + uprv_memcpy2(getFieldPtr(), other.getFieldPtr(), sizeof(Field) * capacity); fZero = other.fZero; fLength = other.fLength; @@ -229,12 +249,12 @@ int32_t NumberStringBuilder::prepareForInsertHelper(int32_t index, int32_t count // First copy the prefix and then the suffix, leaving room for the new chars that the // caller wants to insert. // C++ note: memcpy is OK because the src and dest do not overlap. - uprv_memcpy(newChars + newZero, oldChars + oldZero, sizeof(char16_t) * index); - uprv_memcpy(newChars + newZero + index + count, + uprv_memcpy2(newChars + newZero, oldChars + oldZero, sizeof(char16_t) * index); + uprv_memcpy2(newChars + newZero + index + count, oldChars + oldZero + index, sizeof(char16_t) * (fLength - index)); - uprv_memcpy(newFields + newZero, oldFields + oldZero, sizeof(Field) * index); - uprv_memcpy(newFields + newZero + index + count, + uprv_memcpy2(newFields + newZero, oldFields + oldZero, sizeof(Field) * index); + uprv_memcpy2(newFields + newZero + index + count, oldFields + oldZero + index, sizeof(Field) * (fLength - index)); @@ -255,12 +275,12 @@ int32_t NumberStringBuilder::prepareForInsertHelper(int32_t index, int32_t count // C++ note: memmove is required because src and dest may overlap. // First copy the entire string to the location of the prefix, and then move the suffix // to make room for the new chars that the caller wants to insert. - uprv_memmove(oldChars + newZero, oldChars + oldZero, sizeof(char16_t) * fLength); - uprv_memmove(oldChars + newZero + index + count, + uprv_memmove2(oldChars + newZero, oldChars + oldZero, sizeof(char16_t) * fLength); + uprv_memmove2(oldChars + newZero + index + count, oldChars + newZero + index, sizeof(char16_t) * (fLength - index)); - uprv_memmove(oldFields + newZero, oldFields + oldZero, sizeof(Field) * fLength); - uprv_memmove(oldFields + newZero + index + count, + uprv_memmove2(oldFields + newZero, oldFields + oldZero, sizeof(Field) * fLength); + uprv_memmove2(oldFields + newZero + index + count, oldFields + newZero + index, sizeof(Field) * (fLength - index)); diff --git a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp index b135a255804..61290b067b1 100644 --- a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp +++ b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp @@ -210,7 +210,7 @@ void DecimalQuantityTest::testConvertToAccurateDouble() { "-Inf check failed", -INFINITY, DecimalQuantity().setToDouble(-INFINITY).toDouble()); // Generate random doubles - for (int32_t i = 0; i < 1000000; i++) { + for (int32_t i = 0; i < 10000; i++) { uint8_t bytes[8]; for (int32_t j = 0; j < 8; j++) { bytes[j] = static_cast(rand() % 256); diff --git a/icu4c/source/test/intltest/numbertest_patternmodifier.cpp b/icu4c/source/test/intltest/numbertest_patternmodifier.cpp index b3ad038d1ce..4e34597ffe1 100644 --- a/icu4c/source/test/intltest/numbertest_patternmodifier.cpp +++ b/icu4c/source/test/intltest/numbertest_patternmodifier.cpp @@ -88,7 +88,7 @@ void PatternModifierTest::testMutableEqualsImmutable() { NumberStringBuilder nsb2; MicroProps micros2; - ImmutablePatternModifier *immutable = mod.createImmutable(status); + LocalPointer immutable(mod.createImmutable(status)); immutable->applyToMicros(micros2, fq); micros2.modMiddle->apply(nsb2, 0, 0, status); assertSuccess("Spot 4", status);