From dd506783e6fff467b8cf940a7a20ebabf18a041c Mon Sep 17 00:00:00 2001 From: Travis Keep Date: Thu, 11 Oct 2012 18:52:55 +0000 Subject: [PATCH] ICU-9598 Respond to Markus' comments on C GenderInfo X-SVN-Rev: 32600 --- icu4c/source/i18n/gender.cpp | 37 +++++++------ icu4c/source/i18n/unicode/gender.h | 17 +++--- icu4c/source/i18n/unicode/ugender.h | 16 +++++- icu4c/source/test/cintltst/Makefile.in | 6 +-- icu4c/source/test/cintltst/calltest.c | 5 +- icu4c/source/test/cintltst/cgendtst.c | 52 +++++++++++++++++++ icu4c/source/test/cintltst/cintltst.vcxproj | 3 +- .../test/cintltst/cintltst.vcxproj.filters | 5 +- icu4c/source/test/intltest/genderinfotest.cpp | 50 +++++------------- 9 files changed, 118 insertions(+), 73 deletions(-) create mode 100644 icu4c/source/test/cintltst/cgendtst.c diff --git a/icu4c/source/i18n/gender.cpp b/icu4c/source/i18n/gender.cpp index 9dc5ff1f69f..8e0f4f3bfe1 100644 --- a/icu4c/source/i18n/gender.cpp +++ b/icu4c/source/i18n/gender.cpp @@ -21,6 +21,7 @@ #include "unicode/ugender.h" #include "unicode/ures.h" +#include "cmemory.h" #include "cstring.h" #include "mutex.h" #include "ucln_in.h" @@ -52,6 +53,10 @@ static UBool U_CALLCONV gender_cleanup(void) { return TRUE; } +static void U_CALLCONV deleteChars(void* chars) { + uprv_free(chars); +} + U_CDECL_END U_NAMESPACE_BEGIN @@ -75,29 +80,29 @@ const GenderInfo* GenderInfo::getInstance(const Locale& locale, UErrorCode& stat if (needed) { Mutex lock(&gGenderMetaLock); if (gGenderInfoCache == NULL) { - gGenderInfoCache = uhash_open(uhash_hashChars, uhash_compareChars, NULL, &status); - if (U_FAILURE(status)) { - return NULL; - } gObjs = new GenderInfo[GENDER_STYLE_LENGTH]; if (gObjs == NULL) { status = U_MEMORY_ALLOCATION_ERROR; - uhash_close(gGenderInfoCache); - gGenderInfoCache = NULL; return NULL; } for (int i = 0; i < GENDER_STYLE_LENGTH; i++) { gObjs[i]._style = i; } + gGenderInfoCache = uhash_open(uhash_hashChars, uhash_compareChars, NULL, &status); + if (U_FAILURE(status)) { + delete [] gObjs; + return NULL; + } + uhash_setKeyDeleter(gGenderInfoCache, deleteChars); ucln_i18n_registerCleanup(UCLN_I18N_GENDERINFO, gender_cleanup); } } - GenderInfo* result = NULL; + const GenderInfo* result = NULL; const char* key = locale.getName(); { Mutex lock(&gGenderMetaLock); - result = (GenderInfo*) uhash_get(gGenderInfoCache, key); + result = (const GenderInfo*) uhash_get(gGenderInfoCache, key); } if (result) { return result; @@ -114,8 +119,10 @@ const GenderInfo* GenderInfo::getInstance(const Locale& locale, UErrorCode& stat if (temp) { result = temp; } else { - uhash_put(gGenderInfoCache, (void*) key, result, &status); + char* keyDup = uprv_strdup(key); + uhash_put(gGenderInfoCache, keyDup, (void*) result, &status); if (U_FAILURE(status)) { + uprv_free(keyDup); return NULL; } } @@ -123,7 +130,7 @@ const GenderInfo* GenderInfo::getInstance(const Locale& locale, UErrorCode& stat return result; } -GenderInfo* GenderInfo::loadInstance(const Locale& locale, UErrorCode& status) { +const GenderInfo* GenderInfo::loadInstance(const Locale& locale, UErrorCode& status) { LocalUResourceBundlePointer rb( ures_openDirect(NULL, "genderList", &status)); if (U_FAILURE(status)) { @@ -153,13 +160,13 @@ GenderInfo* GenderInfo::loadInstance(const Locale& locale, UErrorCode& status) { } char type_str[256]; u_UCharsToChars(s, type_str, resLen + 1); - if (!uprv_strcmp(type_str, gNeutralStr)) { + if (uprv_strcmp(type_str, gNeutralStr) == 0) { return &gObjs[NEUTRAL]; } - if (!uprv_strcmp(type_str, gMixedNeutralStr)) { + if (uprv_strcmp(type_str, gMixedNeutralStr) == 0) { return &gObjs[MIXED_NEUTRAL]; } - if (!uprv_strcmp(type_str, gMailTaintsStr)) { + if (uprv_strcmp(type_str, gMailTaintsStr) == 0) { return &gObjs[MALE_TAINTS]; } return &gObjs[NEUTRAL]; @@ -232,11 +239,11 @@ U_NAMESPACE_END U_CAPI const UGenderInfo* U_EXPORT2 ugender_getInstance(const char* locale, UErrorCode* status) { - return (UGenderInfo*) icu::GenderInfo::getInstance(icu::Locale::createFromName(locale), *status); + return (const UGenderInfo*) icu::GenderInfo::getInstance(locale, *status); } U_CAPI UGender U_EXPORT2 -ugender_getListGender(const UGenderInfo* genderInfo, UGender* genders, int32_t size, UErrorCode* status) { +ugender_getListGender(const UGenderInfo* genderInfo, const UGender* genders, int32_t size, UErrorCode* status) { return ((const icu::GenderInfo *)genderInfo)->getListGender(genders, size, *status); } diff --git a/icu4c/source/i18n/unicode/gender.h b/icu4c/source/i18n/unicode/gender.h index 6732052722c..3b2ad006e67 100644 --- a/icu4c/source/i18n/unicode/gender.h +++ b/icu4c/source/i18n/unicode/gender.h @@ -18,12 +18,6 @@ #include "unicode/utypes.h" -/** - * \file - * \brief C++ API: The purpose of this API is to compute the gender of a list as - * a whole given the gender of each element. - */ - #if !UCONFIG_NO_FORMATTING #include "unicode/locid.h" @@ -34,6 +28,11 @@ class GenderInfoTest; U_NAMESPACE_BEGIN +/** + * GenderInfo computes the gender of a list as a whole given the gender of + * each element. + * @draft ICU 50 + */ class U_I18N_API GenderInfo : public UObject { public: @@ -80,21 +79,17 @@ private: /** * No "poor man's RTTI" - * - * @draft ICU 50 */ virtual UClassID getDynamicClassID() const; /** * Copy constructor. One object per locale invariant. Clients * must never copy GenderInfo objects. - * @draft ICU 50 */ GenderInfo(const GenderInfo& other); /** * Assignment operator. Not applicable to immutable objects. - * @draft ICU 50 */ GenderInfo& operator=(const GenderInfo&); @@ -106,7 +101,7 @@ private: static const GenderInfo* getMaleTaintsInstance(); - static GenderInfo* loadInstance(const Locale& locale, UErrorCode& status); + static const GenderInfo* loadInstance(const Locale& locale, UErrorCode& status); friend class ::GenderInfoTest; }; diff --git a/icu4c/source/i18n/unicode/ugender.h b/icu4c/source/i18n/unicode/ugender.h index f5701dafe83..38a40eca366 100644 --- a/icu4c/source/i18n/unicode/ugender.h +++ b/icu4c/source/i18n/unicode/ugender.h @@ -26,8 +26,20 @@ * @draft ICU 50 */ enum UGender { + /** + * Male gender. + * @draft ICU 50 + */ UGENDER_MALE, + /** + * Female gender. + * @draft ICU 50 + */ UGENDER_FEMALE, + /** + * Neutral gender. + * @draft ICU 50 + */ UGENDER_OTHER }; /** @@ -46,7 +58,7 @@ typedef struct UGenderInfo UGenderInfo; * Opens a new UGenderInfo object given locale. * @param locale The locale for which the rules are desired. * @return A UGenderInfo for the specified locale, or NULL if an error occurred. - * @stable ICU 4.8 + * @draft ICU 50 */ U_STABLE const UGenderInfo* U_EXPORT2 ugender_getInstance(const char *locale, UErrorCode *status); @@ -62,7 +74,7 @@ ugender_getInstance(const char *locale, UErrorCode *status); * @draft ICU 50 */ U_DRAFT UGender U_EXPORT2 -ugender_getListGender(const UGenderInfo* genderinfo, UGender *genders, int32_t size, UErrorCode *status); +ugender_getListGender(const UGenderInfo* genderinfo, const UGender *genders, int32_t size, UErrorCode *status); #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/cintltst/Makefile.in b/icu4c/source/test/cintltst/Makefile.in index 6be462522d3..1bf67cee834 100644 --- a/icu4c/source/test/cintltst/Makefile.in +++ b/icu4c/source/test/cintltst/Makefile.in @@ -1,6 +1,6 @@ #****************************************************************************** # -# Copyright (C) 1999-2011, International Business Machines +# Copyright (C) 1999-2012, International Business Machines # Corporation and others. All Rights Reserved. # #****************************************************************************** @@ -49,7 +49,8 @@ ncnvfbts.o ncnvtst.o putiltst.o cstrtest.o udatpg_test.o utf8tst.o \ stdnmtst.o usrchtst.o custrtrn.o sorttest.o trietest.o trie2test.o usettest.o \ uenumtst.o utmstest.o currtest.o \ idnatest.o nfsprep.o spreptst.o sprpdata.o \ -hpmufn.o tracetst.o reapits.o utexttst.o ucsdetst.o spooftest.o +hpmufn.o tracetst.o reapits.o utexttst.o ucsdetst.o spooftest.o \ +cgendtst.o DEPS = $(OBJECTS:.o=.d) @@ -113,4 +114,3 @@ ifneq ($(patsubst %install,,$(MAKECMDGOALS)),) endif endif endif - diff --git a/icu4c/source/test/cintltst/calltest.c b/icu4c/source/test/cintltst/calltest.c index 120fecfc7b2..d6d0efe3ffe 100644 --- a/icu4c/source/test/cintltst/calltest.c +++ b/icu4c/source/test/cintltst/calltest.c @@ -1,6 +1,6 @@ /******************************************************************** * COPYRIGHT: - * Copyright (c) 1996-2010, International Business Machines Corporation and + * Copyright (c) 1996-2012, International Business Machines Corporation and * others. All Rights Reserved. ********************************************************************/ /******************************************************************************** @@ -39,6 +39,7 @@ void addUTextTest(TestNode** root); void addUCsdetTest(TestNode** root); void addCnvSelTest(TestNode** root); void addUSpoofTest(TestNode** root); +void addGendInfoForTest(TestNode** root); void addAllTests(TestNode** root) { @@ -79,5 +80,5 @@ void addAllTests(TestNode** root) addUSpoofTest(root); #endif addPUtilTest(root); + addGendInfoForTest(root); } - diff --git a/icu4c/source/test/cintltst/cgendtst.c b/icu4c/source/test/cintltst/cgendtst.c new file mode 100644 index 00000000000..8cbf838de15 --- /dev/null +++ b/icu4c/source/test/cintltst/cgendtst.c @@ -0,0 +1,52 @@ +/******************************************************************** + * COPYRIGHT: + * Copyright (c) 1997-2012, International Business Machines Corporation and + * others. All Rights Reserved. + ********************************************************************/ +/******************************************************************************** +* +* File CGENDTST.C +********************************************************************************* +*/ + +/* C API TEST FOR GENDER INFO */ + +#include "unicode/utypes.h" + +#if !UCONFIG_NO_FORMATTING + +#include "cintltst.h" +#include "unicode/ugender.h" + +static const UGender kAllFemale[] = {UGENDER_FEMALE, UGENDER_FEMALE}; + +#define LENGTH(arr) (sizeof(arr)/sizeof(arr[0])) + +void addGendInfoForTest(TestNode** root); +static void TestGenderInfo(void); + +#define TESTCASE(x) addTest(root, &x, "tsformat/cgendtst/" #x) + +void addGendInfoForTest(TestNode** root) +{ + TESTCASE(TestGenderInfo); +} + +static void TestGenderInfo(void) { + UErrorCode status = U_ZERO_ERROR; + const UGenderInfo* actual_gi = ugender_getInstance("fr_CA", &status); + if (U_FAILURE(status)) { + log_err("Fail to create UGenderInfo - %s\n", u_errorName(status)); + return; + } + UGender actual = ugender_getListGender(actual_gi, kAllFemale, LENGTH(kAllFemale), &status); + if (U_FAILURE(status)) { + log_err("Fail to get gender of list - %s\n", u_errorName(status)); + return; + } + if (actual != UGENDER_FEMALE) { + log_err("Expected UGENDER_FEMALE got %d\n", actual); + } +} + +#endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/cintltst/cintltst.vcxproj b/icu4c/source/test/cintltst/cintltst.vcxproj index 234e24a8892..ccd56b47c42 100644 --- a/icu4c/source/test/cintltst/cintltst.vcxproj +++ b/icu4c/source/test/cintltst/cintltst.vcxproj @@ -281,6 +281,7 @@ + @@ -376,4 +377,4 @@ - \ No newline at end of file + diff --git a/icu4c/source/test/cintltst/cintltst.vcxproj.filters b/icu4c/source/test/cintltst/cintltst.vcxproj.filters index 0920aa3162a..84f4d735417 100644 --- a/icu4c/source/test/cintltst/cintltst.vcxproj.filters +++ b/icu4c/source/test/cintltst/cintltst.vcxproj.filters @@ -186,6 +186,9 @@ formatting + + formatting + formatting @@ -402,4 +405,4 @@ sprep & idna - \ No newline at end of file + diff --git a/icu4c/source/test/intltest/genderinfotest.cpp b/icu4c/source/test/intltest/genderinfotest.cpp index 1253d619fea..b6ba6eb0973 100644 --- a/icu4c/source/test/intltest/genderinfotest.cpp +++ b/icu4c/source/test/intltest/genderinfotest.cpp @@ -14,17 +14,17 @@ #define LENGTHOF(array) (int32_t)(sizeof(array) / sizeof((array)[0])) -UGender kSingleFemale[] = {UGENDER_FEMALE}; -UGender kSingleMale[] = {UGENDER_MALE}; -UGender kSingleOther[] = {UGENDER_OTHER}; +static const UGender kSingleFemale[] = {UGENDER_FEMALE}; +static const UGender kSingleMale[] = {UGENDER_MALE}; +static const UGender kSingleOther[] = {UGENDER_OTHER}; -UGender kAllFemale[] = {UGENDER_FEMALE, UGENDER_FEMALE}; -UGender kAllMale[] = {UGENDER_MALE, UGENDER_MALE}; -UGender kAllOther[] = {UGENDER_OTHER, UGENDER_OTHER}; +static const UGender kAllFemale[] = {UGENDER_FEMALE, UGENDER_FEMALE}; +static const UGender kAllMale[] = {UGENDER_MALE, UGENDER_MALE}; +static const UGender kAllOther[] = {UGENDER_OTHER, UGENDER_OTHER}; -UGender kFemaleMale[] = {UGENDER_FEMALE, UGENDER_MALE}; -UGender kFemaleOther[] = {UGENDER_FEMALE, UGENDER_OTHER}; -UGender kMaleOther[] = {UGENDER_MALE, UGENDER_OTHER}; +static const UGender kFemaleMale[] = {UGENDER_FEMALE, UGENDER_MALE}; +static const UGender kFemaleOther[] = {UGENDER_FEMALE, UGENDER_OTHER}; +static const UGender kMaleOther[] = {UGENDER_MALE, UGENDER_OTHER}; class GenderInfoTest : public IntlTest { @@ -36,7 +36,6 @@ public: private: void TestGetListGender(); void TestFallback(); - void TestCApi(); void check(UGender expected_neutral, UGender expected_mixed, UGender expected_taints, const UGender* genderList, int32_t listLength); void checkLocale(const Locale& locale, UGender expected, const UGender* genderList, int32_t listLength); }; @@ -48,17 +47,13 @@ void GenderInfoTest::runIndexedTest(int32_t index, UBool exec, const char *&name TESTCASE_AUTO_BEGIN; TESTCASE_AUTO(TestGetListGender); TESTCASE_AUTO(TestFallback); - TESTCASE_AUTO(TestCApi); TESTCASE_AUTO_END; } void GenderInfoTest::TestGetListGender() { check(UGENDER_OTHER, UGENDER_OTHER, UGENDER_OTHER, NULL, 0); - // JAVA version always returns OTHER if gender style is NEUTRAL. Is this - // really correct? check(UGENDER_OTHER, UGENDER_FEMALE, UGENDER_FEMALE, kSingleFemale, LENGTHOF(kSingleFemale)); check(UGENDER_OTHER, UGENDER_MALE, UGENDER_MALE, kSingleMale, LENGTHOF(kSingleMale)); - // JAVA version has MALE_TAINTS return OTHER for {OTHER}. Is this really correct? check(UGENDER_OTHER, UGENDER_OTHER, UGENDER_OTHER, kSingleOther, LENGTHOF(kSingleOther)); check(UGENDER_OTHER, UGENDER_FEMALE, UGENDER_FEMALE, kAllFemale, LENGTHOF(kAllFemale)); @@ -72,7 +67,7 @@ void GenderInfoTest::TestGetListGender() { void GenderInfoTest::TestFallback() { UErrorCode status = U_ZERO_ERROR; - const GenderInfo* actual = GenderInfo::getInstance(Locale::createFromName("xx"), status); + const GenderInfo* actual = GenderInfo::getInstance("xx", status); if (U_FAILURE(status)) { errcheckln(status, "Fail to create GenderInfo - %s", u_errorName(status)); return; @@ -81,7 +76,7 @@ void GenderInfoTest::TestFallback() { if (expected != actual) { errln("For Neutral, expected %d got %d", expected, actual); } - actual = GenderInfo::getInstance(Locale::createFromName("fr_CA"), status); + actual = GenderInfo::getInstance("fr_CA", status); if (U_FAILURE(status)) { errcheckln(status, "Fail to create GenderInfo - %s", u_errorName(status)); return; @@ -92,31 +87,10 @@ void GenderInfoTest::TestFallback() { } } -void GenderInfoTest::TestCApi() { - UErrorCode status = U_ZERO_ERROR; - const UGenderInfo* actual_gi = ugender_getInstance("fr_CA", &status); - if (U_FAILURE(status)) { - errcheckln(status, "Fail to create UGenderInfo - %s", u_errorName(status)); - return; - } - const UGenderInfo* expected_gi = (const UGenderInfo*) GenderInfo::getMaleTaintsInstance(); - if (expected_gi != actual_gi) { - errln("Expected UGenderInfo %d got %d", expected_gi, actual_gi); - } - UGender actual = ugender_getListGender(actual_gi, kAllFemale, LENGTHOF(kAllFemale), &status); - if (U_FAILURE(status)) { - errcheckln(status, "Fail to create UGenderInfo - %s", u_errorName(status)); - return; - } - if (actual != UGENDER_FEMALE) { - errln("Expected UGENDER_FEMALE got %d", actual); - } -} - void GenderInfoTest::check( UGender expected_neutral, UGender expected_mixed, UGender expected_taints, const UGender* genderList, int32_t listLength) { checkLocale(Locale::getUS(), expected_neutral, genderList, listLength); - checkLocale(Locale::createFromName("is"), expected_mixed, genderList, listLength); + checkLocale("is", expected_mixed, genderList, listLength); checkLocale(Locale::getFrench(), expected_taints, genderList, listLength); } -- 2.40.0