Skip to content

Commit

Permalink
ICU-22520 Standardize return on error for all locale functions.
Browse files Browse the repository at this point in the history 
· No function should do anything if an error has already occurred.
· On error, a value of 0, nullptr, {}, etc., should be returned.
· Values shouldn't have overloaded meanings (eg. index or found).
· Values that are never used should not be returned at all.
  • Loading branch information
@roubert
roubert committed Feb 29, 2024
1 parent 35353f2 commit 929cd9b
Show file tree
Hide file tree
Showing 17 changed files with 361 additions and 359 deletions.
2 changes: 2 additions & 0 deletions icu4c/source/common/localebuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ _copyExtensions(const Locale& from, icu::StringEnumeration *keywords,
void
_clearUAttributesAndKeyType(Locale& locale, UErrorCode& errorCode)
{
if (U_FAILURE(errorCode)) { return; }
// Clear Unicode attributes
locale.setKeywordValue(kAttributeKey, "", errorCode);

Expand All @@ -218,6 +219,7 @@ _clearUAttributesAndKeyType(Locale& locale, UErrorCode& errorCode)
void
_setUnicodeExtensions(Locale& locale, const CharString& value, UErrorCode& errorCode)
{
if (U_FAILURE(errorCode)) { return; }
// Add the unicode extensions to extensions_
CharString locale_str("und-u-", errorCode);
locale_str.append(value, errorCode);
Expand Down
46 changes: 27 additions & 19 deletions icu4c/source/common/localematcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// localematcher.cpp
// created: 2019may08 Markus W. Scherer

#include <optional>

#include "unicode/utypes.h"
#include "unicode/localebuilder.h"
#include "unicode/localematcher.h"
Expand Down Expand Up @@ -605,10 +607,11 @@ class LocaleLsrIterator {

const Locale *LocaleMatcher::getBestMatch(const Locale &desiredLocale, UErrorCode &errorCode) const {
if (U_FAILURE(errorCode)) { return nullptr; }
int32_t suppIndex = getBestSuppIndex(
std::optional<int32_t> suppIndex = getBestSuppIndex(
getMaximalLsrOrUnd(likelySubtags, desiredLocale, errorCode),
nullptr, errorCode);
return U_SUCCESS(errorCode) && suppIndex >= 0 ? supportedLocales[suppIndex] : defaultLocale;
return U_SUCCESS(errorCode) && suppIndex.has_value() ? supportedLocales[suppIndex.value()]
: defaultLocale;
}

const Locale *LocaleMatcher::getBestMatch(Locale::Iterator &desiredLocales,
Expand All @@ -618,12 +621,14 @@ const Locale *LocaleMatcher::getBestMatch(Locale::Iterator &desiredLocales,
return defaultLocale;
}
LocaleLsrIterator lsrIter(likelySubtags, desiredLocales, ULOCMATCH_TEMPORARY_LOCALES);
int32_t suppIndex = getBestSuppIndex(lsrIter.next(errorCode), &lsrIter, errorCode);
return U_SUCCESS(errorCode) && suppIndex >= 0 ? supportedLocales[suppIndex] : defaultLocale;
std::optional<int32_t> suppIndex = getBestSuppIndex(lsrIter.next(errorCode), &lsrIter, errorCode);
return U_SUCCESS(errorCode) && suppIndex.has_value() ? supportedLocales[suppIndex.value()]
: defaultLocale;
}

const Locale *LocaleMatcher::getBestMatchForListString(
StringPiece desiredLocaleList, UErrorCode &errorCode) const {
if (U_FAILURE(errorCode)) { return nullptr; }
LocalePriorityList list(desiredLocaleList, errorCode);
LocalePriorityList::Iterator iter = list.iterator();
return getBestMatch(iter, errorCode);
Expand All @@ -634,13 +639,13 @@ LocaleMatcher::Result LocaleMatcher::getBestMatchResult(
if (U_FAILURE(errorCode)) {
return Result(nullptr, defaultLocale, -1, -1, false);
}
int32_t suppIndex = getBestSuppIndex(
std::optional<int32_t> suppIndex = getBestSuppIndex(
getMaximalLsrOrUnd(likelySubtags, desiredLocale, errorCode),
nullptr, errorCode);
if (U_FAILURE(errorCode) || suppIndex < 0) {
if (U_FAILURE(errorCode) || !suppIndex.has_value()) {
return Result(nullptr, defaultLocale, -1, -1, false);
} else {
return Result(&desiredLocale, supportedLocales[suppIndex], 0, suppIndex, false);
return Result(&desiredLocale, supportedLocales[suppIndex.value()], 0, suppIndex.value(), false);
}
}

Expand All @@ -650,18 +655,19 @@ LocaleMatcher::Result LocaleMatcher::getBestMatchResult(
return Result(nullptr, defaultLocale, -1, -1, false);
}
LocaleLsrIterator lsrIter(likelySubtags, desiredLocales, ULOCMATCH_TEMPORARY_LOCALES);
int32_t suppIndex = getBestSuppIndex(lsrIter.next(errorCode), &lsrIter, errorCode);
if (U_FAILURE(errorCode) || suppIndex < 0) {
std::optional<int32_t> suppIndex = getBestSuppIndex(lsrIter.next(errorCode), &lsrIter, errorCode);
if (U_FAILURE(errorCode) || !suppIndex.has_value()) {
return Result(nullptr, defaultLocale, -1, -1, false);
} else {
return Result(lsrIter.orphanRemembered(), supportedLocales[suppIndex],
lsrIter.getBestDesiredIndex(), suppIndex, true);
return Result(lsrIter.orphanRemembered(), supportedLocales[suppIndex.value()],
lsrIter.getBestDesiredIndex(), suppIndex.value(), true);
}
}

int32_t LocaleMatcher::getBestSuppIndex(LSR desiredLSR, LocaleLsrIterator *remainingIter,
UErrorCode &errorCode) const {
if (U_FAILURE(errorCode)) { return -1; }
std::optional<int32_t> LocaleMatcher::getBestSuppIndex(LSR desiredLSR,
LocaleLsrIterator *remainingIter,
UErrorCode &errorCode) const {
if (U_FAILURE(errorCode)) { return std::nullopt; }
int32_t desiredIndex = 0;
int32_t bestSupportedLsrIndex = -1;
for (int32_t bestShiftedDistance = LocaleDistance::shiftDistance(thresholdDistance);;) {
Expand All @@ -684,7 +690,7 @@ int32_t LocaleMatcher::getBestSuppIndex(LSR desiredLSR, LocaleLsrIterator *remai
bestShiftedDistance = LocaleDistance::getShiftedDistance(bestIndexAndDistance);
if (remainingIter != nullptr) {
remainingIter->rememberCurrent(desiredIndex, errorCode);
if (U_FAILURE(errorCode)) { return -1; }
if (U_FAILURE(errorCode)) { return std::nullopt; }
}
bestSupportedLsrIndex = LocaleDistance::getIndex(bestIndexAndDistance);
}
Expand All @@ -695,20 +701,21 @@ int32_t LocaleMatcher::getBestSuppIndex(LSR desiredLSR, LocaleLsrIterator *remai
break;
}
desiredLSR = remainingIter->next(errorCode);
if (U_FAILURE(errorCode)) { return -1; }
if (U_FAILURE(errorCode)) { return std::nullopt; }
++desiredIndex;
}
if (bestSupportedLsrIndex < 0) {
// no good match
return -1;
return std::nullopt;
}
return supportedIndexes[bestSupportedLsrIndex];
}

UBool LocaleMatcher::isMatch(const Locale &desired, const Locale &supported,
UErrorCode &errorCode) const {
if (U_FAILURE(errorCode)) { return false; }
LSR suppLSR = getMaximalLsrOrUnd(likelySubtags, supported, errorCode);
if (U_FAILURE(errorCode)) { return 0; }
if (U_FAILURE(errorCode)) { return false; }
const LSR *pSuppLSR = &suppLSR;
int32_t indexAndDistance = localeDistance.getBestIndexAndDistance(
getMaximalLsrOrUnd(likelySubtags, desired, errorCode),
Expand All @@ -718,9 +725,10 @@ UBool LocaleMatcher::isMatch(const Locale &desired, const Locale &supported,
}

double LocaleMatcher::internalMatch(const Locale &desired, const Locale &supported, UErrorCode &errorCode) const {
if (U_FAILURE(errorCode)) { return 0.; }
// Returns the inverse of the distance: That is, 1-distance(desired, supported).
LSR suppLSR = getMaximalLsrOrUnd(likelySubtags, supported, errorCode);
if (U_FAILURE(errorCode)) { return 0; }
if (U_FAILURE(errorCode)) { return 0.; }
const LSR *pSuppLSR = &suppLSR;
int32_t indexAndDistance = localeDistance.getBestIndexAndDistance(
getMaximalLsrOrUnd(likelySubtags, desired, errorCode),
Expand Down
14 changes: 8 additions & 6 deletions icu4c/source/common/locavailable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,9 @@ icu::UInitOnce ginstalledLocalesInitOnce {};
class AvailableLocalesSink : public ResourceSink {
public:
void put(const char *key, ResourceValue &value, UBool /*noFallback*/, UErrorCode &status) override {
if (U_FAILURE(status)) { return; }
ResourceTable resIndexTable = value.getTable(status);
if (U_FAILURE(status)) {
return;
}
if (U_FAILURE(status)) { return; }
for (int32_t i = 0; resIndexTable.getKeyAndValue(i, key, value); ++i) {
ULocAvailableType type;
if (uprv_strcmp(key, "InstalledLocales") == 0) {
Expand Down Expand Up @@ -138,7 +137,8 @@ class AvailableLocalesStringEnumeration : public StringEnumeration {
AvailableLocalesStringEnumeration(ULocAvailableType type) : fType(type) {
}

const char* next(int32_t *resultLength, UErrorCode&) override {
const char* next(int32_t *resultLength, UErrorCode &status) override {
if (U_FAILURE(status)) { return nullptr; }
ULocAvailableType actualType = fType;
int32_t actualIndex = fIndex++;

Expand Down Expand Up @@ -170,11 +170,13 @@ class AvailableLocalesStringEnumeration : public StringEnumeration {
return result;
}

void reset(UErrorCode&) override {
void reset(UErrorCode &status) override {
if (U_FAILURE(status)) { return; }
fIndex = 0;
}

int32_t count(UErrorCode&) const override {
int32_t count(UErrorCode &status) const override {
if (U_FAILURE(status)) { return 0; }
if (fType == ULOC_AVAILABLE_WITH_LEGACY_ALIASES) {
return gAvailableLocaleCounts[ULOC_AVAILABLE_DEFAULT]
+ gAvailableLocaleCounts[ULOC_AVAILABLE_ONLY_LEGACY_ALIASES];
Expand Down
8 changes: 3 additions & 5 deletions icu4c/source/common/locdispnames.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ _getStringOrCopyKey(const char *path, const char *locale,
const char *substitute,
char16_t *dest, int32_t destCapacity,
UErrorCode &errorCode) {
if (U_FAILURE(errorCode)) { return 0; }
const char16_t *s = nullptr;
int32_t length = 0;

Expand Down Expand Up @@ -368,14 +369,10 @@ _getDisplayNameForComponent(const char *locale,
UDisplayNameGetter *getter,
const char *tag,
UErrorCode &errorCode) {
if (U_FAILURE(errorCode)) { return 0; }
UErrorCode localStatus;
const char* root = nullptr;

/* argument checking */
if (U_FAILURE(errorCode)) {
return 0;
}

if(destCapacity<0 || (destCapacity>0 && dest==nullptr)) {
errorCode = U_ILLEGAL_ARGUMENT_ERROR;
return 0;
Expand Down Expand Up @@ -422,6 +419,7 @@ uloc_getDisplayScript(const char* locale,
char16_t *dest, int32_t destCapacity,
UErrorCode *pErrorCode)
{
if (U_FAILURE(*pErrorCode)) { return 0; }
UErrorCode err = U_ZERO_ERROR;
int32_t res = _getDisplayNameForComponent(locale, displayLocale, dest, destCapacity,
ulocimp_getScript, _kScriptsStandAlone, err);
Expand Down
39 changes: 28 additions & 11 deletions icu4c/source/common/locid.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,8 @@ AliasData::loadData(UErrorCode &status)
*/
AliasData*
AliasDataBuilder::build(UErrorCode &status) {
if (U_FAILURE(status)) { return nullptr; }

LocalUResourceBundlePointer metadata(
ures_openDirect(nullptr, "metadata", &status));
LocalUResourceBundlePointer metadataAlias(
Expand Down Expand Up @@ -1061,7 +1063,7 @@ AliasDataBuilder::build(UErrorCode &status) {
*/
class AliasReplacer {
public:
AliasReplacer(UErrorCode status) :
AliasReplacer(UErrorCode& status) :
language(nullptr), script(nullptr), region(nullptr),
extensions(nullptr),
// store value in variants only once
Expand Down Expand Up @@ -1126,12 +1128,12 @@ class AliasReplacer {
}

// Gather fields and generate locale ID into out.
CharString& outputToString(CharString& out, UErrorCode status);
CharString& outputToString(CharString& out, UErrorCode& status);

// Generate the lookup key.
CharString& generateKey(const char* language, const char* region,
const char* variant, CharString& out,
UErrorCode status);
UErrorCode& status);

void parseLanguageReplacement(const char* replacement,
const char*& replaceLanguage,
Expand Down Expand Up @@ -1168,8 +1170,9 @@ class AliasReplacer {
CharString&
AliasReplacer::generateKey(
const char* language, const char* region, const char* variant,
CharString& out, UErrorCode status)
CharString& out, UErrorCode& status)
{
if (U_FAILURE(status)) { return out; }
out.append(language, status);
if (notEmpty(region)) {
out.append(SEP_CHAR, status)
Expand Down Expand Up @@ -1587,8 +1590,9 @@ AliasReplacer::replaceTransformedExtensions(

CharString&
AliasReplacer::outputToString(
CharString& out, UErrorCode status)
CharString& out, UErrorCode& status)
{
if (U_FAILURE(status)) { return out; }
out.append(language, status);
if (notEmpty(script)) {
out.append(SEP_CHAR, status)
Expand Down Expand Up @@ -1778,6 +1782,7 @@ AliasReplacer::replace(const Locale& locale, CharString& out, UErrorCode& status
bool
canonicalizeLocale(const Locale& locale, CharString& out, UErrorCode& status)
{
if (U_FAILURE(status)) { return false; }
AliasReplacer replacer(status);
return replacer.replace(locale, out, status);
}
Expand All @@ -1787,6 +1792,8 @@ canonicalizeLocale(const Locale& locale, CharString& out, UErrorCode& status)
bool
isKnownCanonicalizedLocale(const char* locale, UErrorCode& status)
{
if (U_FAILURE(status)) { return false; }

if ( uprv_strcmp(locale, "c") == 0 ||
uprv_strcmp(locale, "en") == 0 ||
uprv_strcmp(locale, "en_US") == 0) {
Expand Down Expand Up @@ -2453,7 +2460,8 @@ class KeywordEnumeration : public StringEnumeration {
(int32_t)(current - keywords.data()), status);
}

virtual int32_t count(UErrorCode &/*status*/) const override {
virtual int32_t count(UErrorCode& status) const override {
if (U_FAILURE(status)) { return 0; }
const char *kw = keywords.data();
int32_t result = 0;
while(*kw) {
Expand Down Expand Up @@ -2483,12 +2491,14 @@ class KeywordEnumeration : public StringEnumeration {
}

virtual const UnicodeString* snext(UErrorCode& status) override {
if (U_FAILURE(status)) { return nullptr; }
int32_t resultLength = 0;
const char *s = next(&resultLength, status);
return setChars(s, resultLength, status);
}

virtual void reset(UErrorCode& /*status*/) override {
virtual void reset(UErrorCode& status) override {
if (U_FAILURE(status)) { return; }
current = keywords.data();
}
};
Expand Down Expand Up @@ -2521,7 +2531,8 @@ class UnicodeKeywordEnumeration : public KeywordEnumeration {
if (resultLength != nullptr) *resultLength = 0;
return nullptr;
}
virtual int32_t count(UErrorCode &/*status*/) const override {
virtual int32_t count(UErrorCode& status) const override {
if (U_FAILURE(status)) { return 0; }
const char *kw = keywords.data();
int32_t result = 0;
while(*kw) {
Expand Down Expand Up @@ -2625,14 +2636,17 @@ void
Locale::getUnicodeKeywordValue(StringPiece keywordName,
ByteSink& sink,
UErrorCode& status) const {
if (U_FAILURE(status)) {
return;
}

// TODO: Remove the need for a const char* to a NUL terminated buffer.
const CharString keywordName_nul(keywordName, status);
if (U_FAILURE(status)) {
return;
}

const char* legacy_key = uloc_toLegacyKey(keywordName_nul.data());

if (legacy_key == nullptr) {
status = U_ILLEGAL_ARGUMENT_ERROR;
return;
Expand Down Expand Up @@ -2705,6 +2719,7 @@ void
Locale::setKeywordValue(StringPiece keywordName,
StringPiece keywordValue,
UErrorCode& status) {
if (U_FAILURE(status)) { return; }
// TODO: Remove the need for a const char* to a NUL terminated buffer.
const CharString keywordName_nul(keywordName, status);
const CharString keywordValue_nul(keywordValue, status);
Expand All @@ -2715,16 +2730,18 @@ void
Locale::setUnicodeKeywordValue(StringPiece keywordName,
StringPiece keywordValue,
UErrorCode& status) {
if (U_FAILURE(status)) {
return;
}

// TODO: Remove the need for a const char* to a NUL terminated buffer.
const CharString keywordName_nul(keywordName, status);
const CharString keywordValue_nul(keywordValue, status);

if (U_FAILURE(status)) {
return;
}

const char* legacy_key = uloc_toLegacyKey(keywordName_nul.data());

if (legacy_key == nullptr) {
status = U_ILLEGAL_ARGUMENT_ERROR;
return;
Expand Down
Loading

0 comments on commit 929cd9b

Please sign in to comment.