Re: msvcrt: Implemented wcscpy_s (try 2)

[Alistair Leslie-Hughes]

Hi,
    Is there any thing wrong with this patch?

Best Regards
 Alistair Leslie-Hughes

"Alistair Leslie-Hughes"  wrote in message 
news:fpljui$nsa$[e-mail]...
> Hi,
> Correct msvcrt.spec file
>
> Changelog:
> msvcrt: Implemented wcscpy_s

--------------------------------------------------------------------------------
> From 6860771cc3fc06b3da63f7c674daf71861593ef3 Mon Sep 17 00:00:00 2001
> From: Alistair Leslie-Hughes 
> Date: Fri, 22 Feb 2008 13:48:01 +1100
> Subject: [PATCH] Implemented wcscpy_s
>
> dlls/msvcrt/msvcrt.spec    |    1 +
> dlls/msvcrt/tests/string.c |   45 
> ++++++++++++++++++++++++++++++++++++++++++++
> dlls/msvcrt/wcs.c          |   45 
> ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 91 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/msvcrt/msvcrt.spec b/dlls/msvcrt/msvcrt.spec
> index d6cffcb..7b821e6 100644
> --- a/dlls/msvcrt/msvcrt.spec
> +++ b/dlls/msvcrt/msvcrt.spec
> @@ -761,6 +761,7 @@
> @ cdecl wcscmp(wstr wstr) ntdll.wcscmp
> @ cdecl wcscoll(wstr wstr) MSVCRT_wcscoll
> @ cdecl wcscpy(ptr wstr) ntdll.wcscpy
> +@ cdecl wcscpy_s(ptr long wstr) MSVCRT_wcscpy_s
> @ cdecl wcscspn(wstr wstr) ntdll.wcscspn
> @ cdecl wcsftime(ptr long wstr ptr) MSVCRT_wcsftime
> @ cdecl wcslen(wstr) ntdll.wcslen
> diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c
> index c6dcd34..b09d204 100644
> --- a/dlls/msvcrt/tests/string.c
> +++ b/dlls/msvcrt/tests/string.c
> @@ -50,6 +50,7 @@ static int* (*pmemcmp)(void *, const void *, size_t n);
> static int (*pstrcpy_s)(char *dst, size_t len, const char *src);
> static int (*pstrcat_s)(char *dst, size_t len, const char *src);
> static int (*p_mbsnbcpy_s)(unsigned char * dst, size_t size, const 
> unsigned char * src, size_t count);
> +static int (*p_wcscpy_s)(wchar_t *wcDest, size_t size, const wchar_t 
> *wcSrc);
>
> #define SETNOFAIL(x,y) x = (void*)GetProcAddress(hMsvcrt,y)
> #define SET(x,y) SETNOFAIL(x,y); ok(x != NULL, "Export '%s' not found\n", 
> y)
> @@ -562,6 +563,47 @@ static void test__mbsnbcpy_s(void)
>        dest[0], dest[1], dest[2], dest[3], dest[4], dest[5], dest[6], 
> dest[7]);
> }
>
> +static void test_wcscpy_s(void)
> +{
> - static const WCHAR szLongText[] = {
>
> 'T','h','i','s','A','L','o','n','g','s','t','r','i','n','g',0 };
> - static WCHAR szDest[18];
> - static WCHAR szDestShort[8];
> - int ret;
> -
> - if(!p_wcscpy_s)
> - {
> -
>    skip("wcscpy_s not found\n");
> -
>    return;
> - }
> -
> - /* Test NULL Dest */
> - ret = p_wcscpy_s(NULL, 16, szLongText);
> - ok(ret == EINVAL, "p_wcscpy_s expect EINVAL got %d\n", ret);
> -
> - /* Test NULL Source */
> - szDest[0] = 'A';
> - ret = p_wcscpy_s(szDest, 16, NULL);
> - ok(ret == EINVAL, "expected EINVAL got %d\n", ret);
> - ok(szDest[0] == 0, "szDest[0] not 0\n");
> -
> - /* Test invalid size */
> - szDest[0] = 'A';
> - ret = p_wcscpy_s(szDest, 0, szLongText);
> - ok(ret == EINVAL, "expected ERANGE got %d\n", ret);
> - ok(szDest[0] == 0, "szDest[0] not 0\n");
> -
> - /* Copy same buffer size */
> - ret = p_wcscpy_s(szDest, 18, szLongText);
> - ok(ret == 0, "expected 0 got %d\n", ret);
> - ok(lstrcmpW(szDest, szLongText) == 0, "szDest != szLongText\n");
> -
> - /* Copy smaller buffer size */
> - szDest[0] = 'A';
> - ret = p_wcscpy_s(szDestShort, 8, szLongText);
> - ok(ret == ERANGE, "expected EINVAL got %d\n", ret);
> - ok(szDestShort[0] == 0, "szDest[0] not 0\n");
>
> +}
> -
>
> START_TEST(string)
> {
>     char mem[100];
> @@ -577,6 +619,7 @@ START_TEST(string)
>     SET(pstrcpy_s,"strcpy_s");
>     SET(pstrcat_s,"strcat_s");
>     SET(p_mbsnbcpy_s,"_mbsnbcpy_s");
> -
>
> SET(p_wcscpy_s,"wcscpy_s");
>
> /* MSVCRT memcpy behaves like memmove for overlapping moves,
>   MFC42 CString::Insert seems to rely on that behaviour */
>
> @@ -599,4 +642,6 @@ START_TEST(string)
>     test_strcpy_s();
>     test_strcat_s();
>     test__mbsnbcpy_s();
> -
> - test_wcscpy_s();
>
> }
> diff --git a/dlls/msvcrt/wcs.c b/dlls/msvcrt/wcs.c
> index 92dd644..66b8907 100644
> --- a/dlls/msvcrt/wcs.c
> +++ b/dlls/msvcrt/wcs.c
> @@ -27,6 +27,8 @@
> #include "wine/unicode.h"
> #include "wine/debug.h"
>
> +#include <msvcrt/errno.h>
> -
>
> WINE_DEFAULT_DEBUG_CHANNEL(msvcrt);
>
> @@ -971,3 +973,46 @@ INT CDECL MSVCRT_iswxdigit( MSVCRT_wchar_t wc )
> {
>     return isxdigitW( wc );
> }
> -
>
> +/*********************************************************************
> -
> - wcscpy_s (MSVCRT.@)
> - */
>
> +INT CDECL MSVCRT_wcscpy_s( MSVCRT_wchar_t* wcDest, size_t numElement, 
> const  MSVCRT_wchar_t *wcSrc)
> +{
> - int size = 0;
> -
> - if(!wcDest)
> -
>    return EINVAL;
> -
> - if(!wcSrc)
> - {
> -
>    wcDest[0] = 0;
> -
>    return EINVAL;
> - }
> -
> - if(numElement <= 0)
> - {
> -
>    wcDest[0] = 0;
> -
> -
>    if(numElement < 0)
> -
>        return ERANGE;
> -
>    else
> -
>        return EINVAL;
> - }
> -
> - size = strlenW(wcSrc) + 1;
> -
> - if(size > numElement)
> - {
> -
>    wcDest[0] = 0;
> -
>    return ERANGE;
> - }
> -
> - if(size > numElement)
> -
>    size = numElement;
> -
> - memcpy( wcDest, wcSrc, size*sizeof(WCHAR) );
> -
> - return 0;
>
> +}
> -
>
> -- 
> 1.5.4.1

--------------------------------------------------------------------------------
>

msvcrt: Implemented wcscpy_s (try 2)

[Alexandre Julliard]

"Alistair Leslie-Hughes"  writes:
> Is there any thing wrong with this patch?
> > - /* Test invalid size */
> > - szDest[0] = 'A';
> > - ret = p_wcscpy_s(szDest, 0, szLongText);
> > - ok(ret == EINVAL, "expected ERANGE got %d\n", ret);
> > - ok(szDest[0] == 0, "szDest[0] not 0\n");
> > -
> > - /* Copy same buffer size */
> > - ret = p_wcscpy_s(szDest, 18, szLongText);
> > - ok(ret == 0, "expected 0 got %d\n", ret);
> > - ok(lstrcmpW(szDest, szLongText) == 0, "szDest != szLongText\n");
> > -
> > - /* Copy smaller buffer size */
> > - szDest[0] = 'A';
> > - ret = p_wcscpy_s(szDestShort, 8, szLongText);
> > - ok(ret == ERANGE, "expected EINVAL got %d\n", ret);
> > - ok(szDestShort[0] == 0, "szDest[0] not 0\n");

The error messages don't match the tests.
> > - if(numElement <= 0)
> > - {
> > -
> >    wcDest[0] = 0;
> > -
> > -
> >    if(numElement < 0)
> > -
> >        return ERANGE;
> > -
> >    else
> > -
> >        return EINVAL;

The variable is unsigned so the comparison is useless. Also its type
should be MSVCRT_size_t.