Address feedback

3 files changed, 54 insertions, 70 deletions

diff --git a/src/corefx/System.Globalization.Native/pal_calendarData.c b/src/corefx/System.Globalization.Native/pal_calendarData.c
index c65da8f418..404c0df220 100644
--- a/src/corefx/System.Globalization.Native/pal_calendarData.c
+++ b/src/corefx/System.Globalization.Native/pal_calendarData.c
@@ -123,10 +123,7 @@ int32_t GlobalizationNative_GetCalendars(
     UEnumeration* pEnum = ucal_getKeywordValuesForLocale("calendar", locale, TRUE, &err);
 
     if (U_FAILURE(err))
-    {
-        uenum_close(pEnum);
         return 0;
-    }
 
     int stringEnumeratorCount = uenum_count(pEnum, &err);
     if (U_FAILURE(err))
@@ -167,10 +164,7 @@ ResultCode GetMonthDayPattern(const char* locale, UChar* sMonthDay, int32_t stri
     UDateTimePatternGenerator* pGenerator = udatpg_open(locale, &err);
 
     if (U_FAILURE(err))
-    {
-        udatpg_close(pGenerator);
         return GetResultCode(err);
-    }
 
     udatpg_getBestPattern(pGenerator, UDAT_MONTH_DAY_UCHAR, -1, sMonthDay, stringCapacity, &err);
 
@@ -240,10 +234,7 @@ bool InvokeCallbackForDatePattern(const char* locale,
     UDateFormat* pFormat = udat_open(UDAT_NONE, style, locale, NULL, 0, NULL, 0, &err);
 
     if (U_FAILURE(err))
-    {
-        unum_close(pFormat);
         return false;
-    }
 
     UErrorCode ignore = U_ZERO_ERROR;
     int32_t patternLen = udat_toPattern(pFormat, false, NULL, 0, &ignore) + 1;
@@ -251,11 +242,11 @@ bool InvokeCallbackForDatePattern(const char* locale,
     UChar* pattern = calloc(patternLen, sizeof(UChar));
     if (pattern == NULL)
     {
+        udat_close(pFormat);
         return false;
     }
 
     udat_toPattern(pFormat, false, pattern, patternLen, &err);
-
     udat_close(pFormat);
 
     if (U_SUCCESS(err))
@@ -283,10 +274,7 @@ bool InvokeCallbackForDateTimePattern(const char* locale,
     UDateTimePatternGenerator* pGenerator = udatpg_open(locale, &err);
 
     if (U_FAILURE(err))
-    {
-        udatpg_close(pGenerator);
         return false;
-    }
 
     UErrorCode ignore = U_ZERO_ERROR;
     int32_t patternLen = udatpg_getBestPattern(pGenerator, patternSkeleton, -1, NULL, 0, &ignore) + 1;
@@ -294,12 +282,13 @@ bool InvokeCallbackForDateTimePattern(const char* locale,
     UChar* bestPattern = calloc(patternLen, sizeof(UChar));
     if (bestPattern == NULL)
     {
+        udatpg_close(pGenerator);
         return false;
     }
 
     udatpg_getBestPattern(pGenerator, patternSkeleton, -1, bestPattern, patternLen, &err);
-
     udatpg_close(pGenerator);
+
     if (U_SUCCESS(err))
     {
         callback(bestPattern, context);
@@ -327,10 +316,7 @@ bool EnumSymbols(const char* locale,
     UDateFormat* pFormat = udat_open(UDAT_DEFAULT, UDAT_DEFAULT, locale, NULL, 0, NULL, 0, &err);
 
     if (U_FAILURE(err))
-    {
-        udat_close(pFormat);
         return false;
-    }
 
     char localeWithCalendarName[ULOC_FULLNAME_CAPACITY];
     strncpy(localeWithCalendarName, locale, ULOC_FULLNAME_CAPACITY);
@@ -363,7 +349,7 @@ bool EnumSymbols(const char* locale,
         if (symbolBuf == NULL)
         {
             udat_close(pFormat);
-            free(symbolBuf);
+            ucal_close(pCalendar);
             return false;
         }
 
@@ -374,6 +360,7 @@ bool EnumSymbols(const char* locale,
         if (U_FAILURE(err))
         {
             udat_close(pFormat);
+            ucal_close(pCalendar);
             free(symbolBuf);
             return false;
         }
@@ -383,6 +370,7 @@ bool EnumSymbols(const char* locale,
     }
 
     udat_close(pFormat);
+    ucal_close(pCalendar);
     return true;
 }
 
@@ -571,10 +559,7 @@ int32_t GlobalizationNative_GetLatestJapaneseEra()
     UCalendar* pCal = ucal_open(NULL, 0, JAPANESE_LOCALE_AND_CALENDAR, UCAL_TRADITIONAL, &err);
 
     if (U_FAILURE(err))
-    {
-        ucal_close(pCal);
         return 0;
-    }
 
     ucal_set(pCal, UCAL_EXTENDED_YEAR, 9999);
     int32_t ret = ucal_get(pCal, UCAL_ERA, &err);
@@ -600,10 +585,7 @@ int32_t GlobalizationNative_GetJapaneseEraStartDate(
     UCalendar* pCal = ucal_open(NULL, 0, JAPANESE_LOCALE_AND_CALENDAR, UCAL_TRADITIONAL, &err);
 
     if (U_FAILURE(err))
-    {
-        ucal_close(pCal);
         return false;
-    }
 
     ucal_set(pCal, UCAL_ERA, era);
     ucal_set(pCal, UCAL_YEAR, 1);
diff --git a/src/corefx/System.Globalization.Native/pal_collation.c b/src/corefx/System.Globalization.Native/pal_collation.c
index 8440e48faa..f642680b00 100644
--- a/src/corefx/System.Globalization.Native/pal_collation.c
+++ b/src/corefx/System.Globalization.Native/pal_collation.c
@@ -34,7 +34,9 @@ int TreeComparer(const void* left, const void* right)
 {
     const TCollatorMap* leftMap = left;
     const TCollatorMap* rightMap = right;
-    return leftMap->key - rightMap->key;
+    if (leftMap->key < rightMap->key) return -1;
+    if (leftMap->key > rightMap->key) return 1;
+    return 0;
 }
 
 // Hiragana character range
@@ -120,11 +122,11 @@ bool IsHalfFullHigherSymbol(UChar character)
 static bool AddItem(UCharList* list, size_t* currentCapacity, const UChar item)
 {
     size_t size = list->size++;
-    if(size >= *currentCapacity)
+    if (size >= *currentCapacity)
     {
         *currentCapacity *= 2;
         UChar* ptr = (UChar*)realloc(list->items, *currentCapacity * sizeof(UChar*));
-        if(ptr == NULL)
+        if (ptr == NULL)
         {
             return false;
         }
@@ -284,6 +286,7 @@ UCollator* CloneCollatorWithOptions(const UCollator* pCollator, int32_t options,
         }
 
         pClonedCollator = ucol_openRules(completeRules, completeRulesLength, UCOL_DEFAULT, strength, NULL, pErr);
+        free(customRules);
     }
 
     if (isIgnoreSymbols)
@@ -349,22 +352,32 @@ bool CanIgnoreAllCollationElements(const UCollator* pColl, const UChar* lpStr, i
 
 }
 
-ResultCode GlobalizationNative_GetSortHandle(const char* lpLocaleName, SortHandle** ppSortHandle)
+void CreateSortHandle(SortHandle** ppSortHandle)
 {
-    assert(ppSortHandle != NULL);
-
     *ppSortHandle = (SortHandle*)malloc(sizeof(SortHandle));
     if ((*ppSortHandle) == NULL)
     {
-        return GetResultCode(U_MEMORY_ALLOCATION_ERROR);
+        return;
     }
 
     (*ppSortHandle)->pRoot = NULL;
     int result = pthread_mutex_init(&(*ppSortHandle)->collatorsLockObject, NULL);
+
     if (result != 0)
     {
         assert(false && "Unexpected pthread_mutex_init return value.");
     }
+}
+
+ResultCode GlobalizationNative_GetSortHandle(const char* lpLocaleName, SortHandle** ppSortHandle)
+{
+    assert(ppSortHandle != NULL);
+
+    CreateSortHandle(ppSortHandle);
+    if ((*ppSortHandle) == NULL)
+    {
+        return GetResultCode(U_MEMORY_ALLOCATION_ERROR);
+    }
 
     UErrorCode err = U_ZERO_ERROR;
 
@@ -372,9 +385,6 @@ ResultCode GlobalizationNative_GetSortHandle(const char* lpLocaleName, SortHandl
 
     if (U_FAILURE(err))
     {
-        if ((*ppSortHandle)->regular != NULL)
-            ucol_close((*ppSortHandle)->regular);
-
         free(*ppSortHandle);
         (*ppSortHandle) = NULL;
     }
@@ -426,6 +436,7 @@ const UCollator* GetCollatorFromSortHandle(SortHandle* pSortHandle, int32_t opti
         }
         else
         {
+            free(map);
             pCollator = (*(TCollatorMap**)entry)->UCollator;
         }
 
@@ -483,11 +494,11 @@ Function:
 IndexOf
 */
 int32_t GlobalizationNative_IndexOf(
-                        SortHandle* pSortHandle, 
-                        const UChar* lpTarget, 
-                        int32_t cwTargetLength, 
-                        const UChar* lpSource, 
-                        int32_t cwSourceLength, 
+                        SortHandle* pSortHandle,
+                        const UChar* lpTarget,
+                        int32_t cwTargetLength,
+                        const UChar* lpSource,
+                        int32_t cwSourceLength,
                         int32_t options,
                         int32_t* pMatchedLength)
 {
@@ -508,8 +519,8 @@ int32_t GlobalizationNative_IndexOf(
             // if the search was successful,
             // we'll try to get the matched string length.
             if(result != USEARCH_DONE && pMatchedLength != NULL)
-            { 
-                *pMatchedLength = usearch_getMatchedLength(pSearch);	
+            {
+                *pMatchedLength = usearch_getMatchedLength(pSearch);
             }
             usearch_close(pSearch);
         }
@@ -523,11 +534,11 @@ Function:
 LastIndexOf
 */
 int32_t GlobalizationNative_LastIndexOf(
-                        SortHandle* pSortHandle, 
-                        const UChar* lpTarget, 
-                        int32_t cwTargetLength, 
-                        const UChar* lpSource, 
-                        int32_t cwSourceLength, 
+                        SortHandle* pSortHandle,
+                        const UChar* lpTarget,
+                        int32_t cwTargetLength,
+                        const UChar* lpSource,
+                        int32_t cwSourceLength,
                         int32_t options)
 {
     static_assert(USEARCH_DONE == -1, "managed side requires -1 for not found");
@@ -624,11 +635,11 @@ int32_t GlobalizationNative_IndexOfOrdinalIgnoreCase(
  Return value is a "Win32 BOOL" (1 = true, 0 = false)
  */
 int32_t GlobalizationNative_StartsWith(
-                        SortHandle* pSortHandle, 
-                        const UChar* lpTarget, 
-                        int32_t cwTargetLength, 
-                        const UChar* lpSource, 
-                        int32_t cwSourceLength, 
+                        SortHandle* pSortHandle,
+                        const UChar* lpTarget,
+                        int32_t cwTargetLength,
+                        const UChar* lpSource,
+                        int32_t cwSourceLength,
                         int32_t options)
 {
     int32_t result = FALSE;
@@ -666,11 +677,11 @@ int32_t GlobalizationNative_StartsWith(
  Return value is a "Win32 BOOL" (1 = true, 0 = false)
  */
 int32_t GlobalizationNative_EndsWith(
-                        SortHandle* pSortHandle, 
-                        const UChar* lpTarget, 
-                        int32_t cwTargetLength, 
-                        const UChar* lpSource, 
-                        int32_t cwSourceLength, 
+                        SortHandle* pSortHandle,
+                        const UChar* lpTarget,
+                        int32_t cwTargetLength,
+                        const UChar* lpSource,
+                        int32_t cwSourceLength,
                         int32_t options)
 {
     int32_t result = FALSE;
@@ -709,11 +720,11 @@ int32_t GlobalizationNative_EndsWith(
 }
 
 int32_t GlobalizationNative_GetSortKey(
-                        SortHandle* pSortHandle, 
-                        const UChar* lpStr, 
-                        int32_t cwStrLength, 
-                        uint8_t* sortKey, 
-                        int32_t cbSortKeyLength, 
+                        SortHandle* pSortHandle,
+                        const UChar* lpStr,
+                        int32_t cwStrLength,
+                        uint8_t* sortKey,
+                        int32_t cbSortKeyLength,
                         int32_t options)
 {
     UErrorCode err = U_ZERO_ERROR;
diff --git a/src/corefx/System.Globalization.Native/pal_localeStringData.c b/src/corefx/System.Globalization.Native/pal_localeStringData.c
index a7256e665b..5f97792eaa 100644
--- a/src/corefx/System.Globalization.Native/pal_localeStringData.c
+++ b/src/corefx/System.Globalization.Native/pal_localeStringData.c
@@ -22,10 +22,7 @@ GetLocaleInfoDecimalFormatSymbol(const char* locale, UNumberFormatSymbol symbol,
     UNumberFormat* pFormat = unum_open(UNUM_DECIMAL, NULL, 0, locale, NULL, &status);
 
     if (U_FAILURE(status))
-    {
-        unum_close(pFormat);
         return status;
-    }
 
     unum_getSymbol(pFormat, symbol, value, valueLength, &status);
 
@@ -66,10 +63,7 @@ UErrorCode GetLocaleInfoAmPm(const char* locale, bool am, UChar* value, int32_t
     UDateFormat* pFormat = udat_open(UDAT_DEFAULT, UDAT_DEFAULT, locale, NULL, 0, NULL, 0, &status);
 
     if (U_FAILURE(status))
-    {
-        udat_close(pFormat);
         return status;
-    }
 
     udat_getSymbols(pFormat, UDAT_AM_PMS, am ? 0 : 1, value, valueLength, &status);
 
@@ -381,10 +375,7 @@ int32_t GlobalizationNative_GetLocaleTimeFormat(
     UDateFormat* pFormat = udat_open(style, UDAT_NONE, locale, NULL, 0, NULL, 0, &err);
 
     if (U_FAILURE(err))
-    {
-        udat_close(pFormat);
         return UErrorCodeToBool(err);
-    }
 
     udat_toPattern(pFormat, false, value, valueLength, &err);