Merge PAL's _wcslwr into _wcslwr_s (#43265)

In (non-palsuite) product code, `_wcslwr` is only used within PAL inside `_wcslwr_unsafe()` method, which is exposed as `_wcslwr_s` for PAL consumers. PR inlines the usage of `_wcslwr` in `_wcslwr_unsafe` and fixes up PAL tests.
dotnet · Oct 26, 2020 · c107fa8 · c107fa8
1 parent c08ac77
commit c107fa8
Show file tree

Hide file tree

Showing 10 changed files with 59 additions and 85 deletions.
diff --git a/src/coreclr/src/pal/inc/mbusafecrt.h b/src/coreclr/src/pal/inc/mbusafecrt.h
@@ -101,6 +101,8 @@ extern int swscanf_s( const WCHAR *string, const WCHAR *format, ... );
 extern errno_t memcpy_s( void * dst, size_t sizeInBytes, const void * src, size_t count ) THROW_DECL;
 extern errno_t memmove_s( void * dst, size_t sizeInBytes, const void * src, size_t count );
 
+extern errno_t _wcslwr_s(char16_t *string, size_t sz);
+
 #ifdef __cplusplus
     }
 #endif

diff --git a/src/coreclr/src/pal/inc/pal.h b/src/coreclr/src/pal/inc/pal.h
@@ -3980,7 +3980,7 @@ PALIMPORT int __cdecl PAL_swscanf(const WCHAR *, const WCHAR *, ...);
 PALIMPORT DLLEXPORT ULONG __cdecl PAL_wcstoul(const WCHAR *, WCHAR **, int);
 PALIMPORT double __cdecl PAL_wcstod(const WCHAR *, WCHAR **);
 
-PALIMPORT WCHAR * __cdecl _wcslwr(WCHAR *);
+PALIMPORT errno_t __cdecl _wcslwr_s(WCHAR *, size_t sz);
 PALIMPORT DLLEXPORT ULONGLONG _wcstoui64(const WCHAR *, WCHAR **, int);
 PALIMPORT DLLEXPORT errno_t __cdecl _i64tow_s(long long, WCHAR *, size_t, int);
 PALIMPORT int __cdecl _wtoi(const WCHAR *);

diff --git a/src/coreclr/src/pal/inc/rt/palrt.h b/src/coreclr/src/pal/inc/rt/palrt.h
@@ -715,7 +715,6 @@ The wrappers below are simple implementations that may not be as robust as compl
 Remember to fix the errcode defintion in safecrt.h.
 */
 
-#define _wcslwr_s _wcslwr_unsafe
 #define swscanf_s swscanf
 
 #define _wfopen_s _wfopen_unsafe
@@ -727,33 +726,11 @@ extern "C++" {
 
 #include <safemath.h>
 
-inline errno_t __cdecl _wcslwr_unsafe(WCHAR *str, size_t sz)
-{
-    size_t fullSize;
-    if(!ClrSafeInt<size_t>::multiply(sz, sizeof(WCHAR), fullSize))
-        return 1;
-    WCHAR *copy = (WCHAR *)malloc(fullSize);
-    if(copy == nullptr)
-        return 1;
-
-    errno_t retCode = wcscpy_s(copy, sz, str);
-    if(retCode) {
-        free(copy);
-        return 1;
-    }
-
-    _wcslwr(copy);
-    wcscpy_s(str, sz, copy);
-    free(copy);
-
-    return 0;
-}
-
 inline int __cdecl _vscprintf_unsafe(const char *_Format, va_list _ArgList)
 {
     int guess = 10;
 
-    for (;;)
+    while (true)
     {
         char *buf = (char *)malloc(guess * sizeof(char));
         if(buf == nullptr)
@@ -797,7 +774,6 @@ inline errno_t __cdecl _fopen_unsafe(PAL_FILE * *ff, const char *fileName, const
 }
 #endif /* __cplusplus */
 
-
 STDAPI_(BOOL) PathAppendW(LPWSTR pszPath, LPCWSTR pszMore);
 STDAPI_(int) PathCommonPrefixW(LPCWSTR pszFile1, LPCWSTR pszFile2, LPWSTR  pszPath);
 PALIMPORT LPWSTR PALAPI PathFindFileNameW(LPCWSTR pPath);

diff --git a/src/coreclr/src/pal/src/CMakeLists.txt b/src/coreclr/src/pal/src/CMakeLists.txt
@@ -199,6 +199,7 @@ set(SOURCES
   safecrt/wcscat_s.cpp
   safecrt/wcscpy_s.cpp
   safecrt/wcslen_s.cpp
+  safecrt/wcslwr_s.cpp
   safecrt/wcsncat_s.cpp
   safecrt/wcsncpy_s.cpp
   safecrt/wcstok_s.cpp

diff --git a/src/coreclr/src/pal/src/cruntime/wchar.cpp b/src/coreclr/src/pal/src/cruntime/wchar.cpp
@@ -36,7 +36,6 @@ Module Name:
 
 SET_DEFAULT_DEBUG_CHANNEL(CRT);
 
-
 /*--
 Function:
   _wtoi
@@ -193,48 +192,6 @@ _wcsicmp(
     return ret;
 }
 
-
-/*++
-Function:
-  _wcslwr
-
-Convert a string to lowercase.
-
-Return Value
-
-Returns a pointer to the converted string. Because the modification is
-done in place, the pointer returned is the same as the pointer passed
-as the input argument. No return value is reserved to indicate an
-error.
-
-Parameter
-
-string   Null-terminated string to convert to lowercase
-
-Remarks
-
---*/
-wchar_16 *
-__cdecl
-_wcslwr(
-        wchar_16 *string)
-{
-    int i;
-
-    PERF_ENTRY(_wcslwr);
-    ENTRY("_wcslwr (string=%p (%S))\n", string?string:W16_NULLSTRING, string?string:W16_NULLSTRING);
-
-    for (i=0 ; string[i] != 0; i++)
-    {
-        string[i] = towlower(string[i]);
-    }
-
-    LOGEXIT("_wcslwr returning wchar_t %p (%S)\n", string?string:W16_NULLSTRING, string?string:W16_NULLSTRING);
-    PERF_EXIT(_wcslwr);
-    return string;
-}
-
-
 /*++
 Function:
   PAL_wcstoul

diff --git a/src/coreclr/src/pal/src/safecrt/wcslwr_s.cpp b/src/coreclr/src/pal/src/safecrt/wcslwr_s.cpp
@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+/***
+*wcslwr_s.cpp - contains _wcslwr_s() routine
+*
+*
+*Purpose:
+*   _wcslwr_s converts, in place, any upper case letters in input string to
+*   lowercase.
+*
+*******************************************************************************/
+
+#include <wctype.h>
+#include <string.h>
+#include <errno.h>
+#include <limits.h>
+#include "internal_securecrt.h"
+
+#include "mbusafecrt_internal.h"
+
+DLLEXPORT errno_t __cdecl _wcslwr_s(char16_t *string, size_t sz)
+{
+    _VALIDATE_RETURN_ERRCODE(string != NULL, EINVAL);
+    size_t length = PAL_wcsnlen(string, sz);
+    if (length >= sz)
+    {
+        _RETURN_DEST_NOT_NULL_TERMINATED(string, sz);
+    }
+
+    for (int i = 0; string[i] != 0; i++)
+    {
+        string[i] = towlower(string[i]);
+    }
+
+    _FILL_STRING(string, sz, length + 1);
+
+    return 0;
+}
diff --git a/src/coreclr/src/pal/tests/palsuite/CMakeLists.txt b/src/coreclr/src/pal/tests/palsuite/CMakeLists.txt
@@ -513,7 +513,7 @@ _add_executable(paltests
   c_runtime/_vsnwprintf_s/test8/test8.cpp
   c_runtime/_vsnwprintf_s/test9/test9.cpp
   c_runtime/_wcsicmp/test1/test1.cpp
-  c_runtime/_wcslwr/test1/test1.cpp
+  c_runtime/_wcslwr_s/test1/test1.cpp
   c_runtime/_wcsnicmp/test1/test1.cpp
   c_runtime/_wfopen/test1/test1.cpp
   c_runtime/_wfopen/test2/test2.cpp

diff --git a/...alsuite/c_runtime/_wcslwr/test1/test1.cpp → ...suite/c_runtime/_wcslwr_s/test1/test1.cpp b/...alsuite/c_runtime/_wcslwr/test1/test1.cpp → ...suite/c_runtime/_wcslwr_s/test1/test1.cpp
@@ -5,9 +5,9 @@
 **
 ** Source:  test1.c
 **
-** Purpose: Using memcmp to check the result, convert a wide character string 
-** with capitals, to all lowercase using this function. Test #1 for the 
-** wcslwr function
+** Purpose: Using memcmp to check the result, convert a wide character string
+** with capitals, to all lowercase using this function. Test #1 for the
+** _wcslwr_s function
 **
 **
 **==========================================================================*/
@@ -16,11 +16,10 @@
 
 /* uses memcmp,wcslen */
 
-PALTEST(c_runtime__wcslwr_test1_paltest_wcslwr_test1, "c_runtime/_wcslwr/test1/paltest_wcslwr_test1")
+PALTEST(c_runtime__wcslwr_s_test1_paltest_wcslwr_s_test1, "c_runtime/_wcslwr_s/test1/paltest_wcslwr_s_test1")
 {
     WCHAR *test_str   = NULL;
     WCHAR *expect_str = NULL;
-    WCHAR *result_str = NULL;
 
     /*
      *  Initialize the PAL and return FAIL if this fails
@@ -29,19 +28,19 @@ PALTEST(c_runtime__wcslwr_test1_paltest_wcslwr_test1, "c_runtime/_wcslwr/test1/p
     {
         return FAIL;
     }
-
-	test_str   = convert("aSdF 1#");
-	expect_str = convert("asdf 1#");
 
-    result_str = _wcslwr(test_str);
-    if (memcmp(result_str, expect_str, wcslen(expect_str)*2 + 2) != 0)
+    test_str   = convert("aSdF 1#");
+    expect_str = convert("asdf 1#");
+
+    errno_t ret = _wcslwr_s(test_str, 8);
+    if (ret != 0 || memcmp(test_str, expect_str, wcslen(expect_str)*2 + 2) != 0)
     {
         Fail ("ERROR: Expected to get \"%s\", got \"%s\".\n",
-                convertC(expect_str), convertC(result_str));
+                convertC(expect_str), convertC(test_str));
     }
 
-	free(result_str);
-	free(expect_str);
+    free(test_str);
+    free(expect_str);
 
     PAL_Terminate();
     return PASS;

diff --git a/src/coreclr/src/pal/tests/palsuite/compilableTests.txt b/src/coreclr/src/pal/tests/palsuite/compilableTests.txt
@@ -445,7 +445,7 @@ c_runtime/_vsnwprintf_s/test7/paltest_vsnwprintf_test7
 c_runtime/_vsnwprintf_s/test8/paltest_vsnwprintf_test8
 c_runtime/_vsnwprintf_s/test9/paltest_vsnwprintf_test9
 c_runtime/_wcsicmp/test1/paltest_wcsicmp_test1
-c_runtime/_wcslwr/test1/paltest_wcslwr_test1
+c_runtime/_wcslwr_s/test1/paltest_wcslwr_s_test1
 c_runtime/_wcsnicmp/test1/paltest_wcsnicmp_test1
 c_runtime/_wfopen/test1/paltest_wfopen_test1
 c_runtime/_wfopen/test2/paltest_wfopen_test2

diff --git a/src/coreclr/src/pal/tests/palsuite/paltestlist.txt b/src/coreclr/src/pal/tests/palsuite/paltestlist.txt
@@ -425,7 +425,7 @@ c_runtime/_vsnwprintf_s/test6/paltest_vsnwprintf_test6
 c_runtime/_vsnwprintf_s/test8/paltest_vsnwprintf_test8
 c_runtime/_vsnwprintf_s/test9/paltest_vsnwprintf_test9
 c_runtime/_wcsicmp/test1/paltest_wcsicmp_test1
-c_runtime/_wcslwr/test1/paltest_wcslwr_test1
+c_runtime/_wcslwr_s/test1/paltest_wcslwr_s_test1
 c_runtime/_wcsnicmp/test1/paltest_wcsnicmp_test1
 c_runtime/_wfopen/test1/paltest_wfopen_test1
 c_runtime/_wfopen/test2/paltest_wfopen_test2