ICU-9598 Respond to Markus' comments on C GenderInfo

X-SVN-Rev: 32600

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);
 }
 

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;
 };
 

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 */
 

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
-

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);
 }
-

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 */

icu4c/source/test/cintltst/cintltst.vcxproj

@@ -281,6 +281,7 @@
     <ClCompile Include="cdtdptst.c" />
     <ClCompile Include="cdtrgtst.c" />
     <ClCompile Include="cformtst.c" />
+    <ClCompile Include="cgendtst.c" />
     <ClCompile Include="cmsgtst.c" />
     <ClCompile Include="cnmdptst.c" />
     <ClCompile Include="cnumtst.c" />
@@ -376,4 +377,4 @@
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
   </ImportGroup>
-</Project>
+</Project>

icu4c/source/test/cintltst/cintltst.vcxproj.filters

@@ -186,6 +186,9 @@
     <ClCompile Include="cformtst.c">
       <Filter>formatting</Filter>
     </ClCompile>
+    <ClCompile Include="cgendtst.c">
+      <Filter>formatting</Filter>
+    </ClCompile>
     <ClCompile Include="cmsgtst.c">
       <Filter>formatting</Filter>
     </ClCompile>
@@ -402,4 +405,4 @@
       <Filter>sprep &amp; idna</Filter>
     </ClInclude>
   </ItemGroup>
-</Project>
+</Project>

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);
 }