msvcrt: memmove_s shouldn't zero its output buffer on error.

dlls/msvcrt/heap.c

@@ -554,22 +554,37 @@ int CDECL memmove_s(void *dest, MSVCRT_size_t numberOfElements, const void *src,
     if(!count)
         return 0;
 
-    if(!dest || !src) {
-        if(dest)
-            memset(dest, 0, numberOfElements);
+    if (!MSVCRT_CHECK_PMT(dest != NULL)) return MSVCRT_EINVAL;
+    if (!MSVCRT_CHECK_PMT(src != NULL)) return MSVCRT_EINVAL;
+    if (!MSVCRT_CHECK_PMT_ERR( count <= numberOfElements, MSVCRT_ERANGE )) return MSVCRT_ERANGE;
 
-        *MSVCRT__errno() = MSVCRT_EINVAL;
+    memmove(dest, src, count);
+    return 0;
+}
+
+/*********************************************************************
+ *		memcpy_s (MSVCRT.@)
+ */
+int CDECL memcpy_s(void *dest, MSVCRT_size_t numberOfElements, const void *src, MSVCRT_size_t count)
+{
+    TRACE("(%p %lu %p %lu)\n", dest, numberOfElements, src, count);
+
+    if(!count)
+        return 0;
+
+    if (!MSVCRT_CHECK_PMT(dest != NULL)) return MSVCRT_EINVAL;
+    if (!MSVCRT_CHECK_PMT(src != NULL))
+    {
+        memset(dest, 0, numberOfElements);
         return MSVCRT_EINVAL;
     }
-
-    if(count > numberOfElements) {
+    if (!MSVCRT_CHECK_PMT_ERR( count <= numberOfElements, MSVCRT_ERANGE ))
+    {
         memset(dest, 0, numberOfElements);
-
-        *MSVCRT__errno() = MSVCRT_ERANGE;
         return MSVCRT_ERANGE;
     }
 
-    memmove(dest, src, count);
+    memcpy(dest, src, count);
     return 0;
 }
 

dlls/msvcrt/msvcrt.spec

@@ -1339,7 +1339,7 @@
 @ cdecl memchr(ptr long long) ntdll.memchr
 @ cdecl memcmp(ptr ptr long) ntdll.memcmp
 @ cdecl memcpy(ptr ptr long) ntdll.memcpy
-@ cdecl memcpy_s(ptr long ptr long) memmove_s
+@ cdecl memcpy_s(ptr long ptr long)
 @ cdecl memmove(ptr ptr long) ntdll.memmove
 @ cdecl memmove_s(ptr long ptr long)
 @ cdecl memset(ptr long long) ntdll.memset

dlls/msvcrt/tests/misc.c

@@ -23,7 +23,6 @@
 #include "msvcrt.h"
 
 static int (__cdecl *prand_s)(unsigned int *);
-static int (__cdecl *pmemcpy_s)(void *, MSVCRT_size_t, void*, MSVCRT_size_t);
 static int (__cdecl *pI10_OUTPUT)(long double, int, int, void*);
 static int (__cdecl *pstrerror_s)(char *, MSVCRT_size_t, int);
 static int (__cdecl *p_get_doserrno)(int *);
@@ -36,7 +35,6 @@ static void init(void)
     HMODULE hmod = GetModuleHandleA("msvcrt.dll");
 
     prand_s = (void *)GetProcAddress(hmod, "rand_s");
-    pmemcpy_s = (void*)GetProcAddress(hmod, "memcpy_s");
     pI10_OUTPUT = (void*)GetProcAddress(hmod, "$I10_OUTPUT");
     pstrerror_s = (void *)GetProcAddress(hmod, "strerror_s");
     p_get_doserrno = (void *)GetProcAddress(hmod, "_get_doserrno");
@@ -65,57 +63,6 @@ static void test_rand_s(void)
     ok(ret == 0, "Expected rand_s to return 0, got %d\n", ret);
 }
 
-static void test_memcpy_s(void)
-{
-    static char data[] = "data\0to\0be\0copied";
-    static char dest[32];
-    int ret;
-
-    if(!pmemcpy_s)
-    {
-        win_skip("memcpy_s is not available\n");
-        return;
-    }
-
-    errno = 0xdeadbeef;
-    ret = pmemcpy_s(NULL, 0, NULL, 0);
-    ok(ret == 0, "ret = %x\n", ret);
-    ok(errno == 0xdeadbeef, "errno = %x\n", errno);
-
-    errno = 0xdeadbeef;
-    dest[0] = 'x';
-    ret = pmemcpy_s(dest, 10, NULL, 0);
-    ok(ret == 0, "ret = %x\n", ret);
-    ok(errno == 0xdeadbeef, "errno = %x\n", errno);
-    ok(dest[0] == 'x', "dest[0] != \'x\'\n");
-
-    errno = 0xdeadbeef;
-    ret = pmemcpy_s(NULL, 10, data, 10);
-    ok(ret == EINVAL, "ret = %x\n", ret);
-    ok(errno == EINVAL, "errno = %x\n", errno);
-
-    errno = 0xdeadbeef;
-    dest[7] = 'x';
-    ret = pmemcpy_s(dest, 10, data, 5);
-    ok(ret == 0, "ret = %x\n", ret);
-    ok(errno == 0xdeadbeef, "errno = %x\n", errno);
-    ok(memcmp(dest, data, 10), "All data copied\n");
-    ok(!memcmp(dest, data, 5), "First five bytes are different\n");
-
-    errno = 0xdeadbeef;
-    ret = pmemcpy_s(data, 10, data, 10);
-    ok(ret == 0, "ret = %x\n", ret);
-    ok(errno == 0xdeadbeef, "errno = %x\n", errno);
-    ok(!memcmp(dest, data, 5), "data was destroyed during overwriting\n");
-
-    errno = 0xdeadbeef;
-    dest[0] = 'x';
-    ret = pmemcpy_s(dest, 5, data, 10);
-    ok(ret == ERANGE, "ret = %x\n", ret);
-    ok(errno == ERANGE, "errno = %x\n", errno);
-    ok(dest[0] == '\0', "dest[0] != \'\\0\'\n");
-}
-
 typedef struct _I10_OUTPUT_data {
     short pos;
     char sign;
@@ -367,7 +314,6 @@ START_TEST(misc)
     init();
 
     test_rand_s();
-    test_memcpy_s();
     test_I10_OUTPUT();
     test_strerror_s();
     test__get_doserrno();

dlls/msvcrt/tests/string.c

@@ -55,6 +55,8 @@ static void __cdecl test_invalid_parameter_handler(const wchar_t *expression,
 #define expect_bin(buf, value, len) { ok(memcmp((buf), value, len) == 0, "Binary buffer mismatch - expected %s, got %s\n", buf_to_string((unsigned char *)value, len, 1), buf_to_string((buf), len, 0)); }
 
 static void* (__cdecl *pmemcpy)(void *, const void *, size_t n);
+static int (__cdecl *p_memcpy_s)(void *, size_t, const void *, size_t);
+static int (__cdecl *p_memmove_s)(void *, size_t, const void *, size_t);
 static int* (__cdecl *pmemcmp)(void *, const void *, size_t n);
 static int (__cdecl *pstrcpy_s)(char *dst, size_t len, const char *src);
 static int (__cdecl *pstrcat_s)(char *dst, size_t len, const char *src);
@@ -497,6 +499,148 @@ static void test_strcpy_s(void)
     ok(ret == EINVAL, "Copying a big string a NULL dest returned %d, expected EINVAL\n", ret);
 }
 
+#define NUMELMS(array) (sizeof(array)/sizeof((array)[0]))
+
+#define okchars(dst, b0, b1, b2, b3, b4, b5, b6, b7) \
+    ok(dst[0] == b0 && dst[1] == b1 && dst[2] == b2 && dst[3] == b3 && \
+       dst[4] == b4 && dst[5] == b5 && dst[6] == b6 && dst[7] == b7, \
+       "Bad result: 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x\n",\
+       dst[0], dst[1], dst[2], dst[3], dst[4], dst[5], dst[6], dst[7])
+
+static void test_memcpy_s(void)
+{
+    static char dest[8];
+    static const char tiny[] = {'T',0,'I','N','Y',0};
+    static const char big[] = {'a','t','o','o','l','o','n','g','s','t','r','i','n','g',0};
+    int ret;
+    if (!p_memcpy_s) {
+        skip("memcpy_s not found\n");
+        return;
+    }
+
+    if (p_set_invalid_parameter_handler)
+        ok(p_set_invalid_parameter_handler(test_invalid_parameter_handler) == NULL,
+            "Invalid parameter handler was already set\n");
+
+    /* Normal */
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memcpy_s(dest, NUMELMS(dest), tiny, NUMELMS(tiny));
+    ok(ret == 0, "Copying a buffer into a big enough destination returned %d, expected 0\n", ret);
+    okchars(dest, tiny[0], tiny[1], tiny[2], tiny[3], tiny[4], tiny[5], 'X', 'X');
+
+    /* Vary source size */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memcpy_s(dest, NUMELMS(dest), big, NUMELMS(big));
+    ok(ret == ERANGE, "Copying a big buffer to a small destination returned %d, expected ERANGE\n", ret);
+    ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno);
+    okchars(dest, 0, 0, 0, 0, 0, 0, 0, 0);
+
+    /* Replace source with NULL */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memcpy_s(dest, NUMELMS(dest), NULL, NUMELMS(tiny));
+    ok(ret == EINVAL, "Copying a NULL source buffer returned %d, expected EINVAL\n", ret);
+    ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno);
+    okchars(dest, 0, 0, 0, 0, 0, 0, 0, 0);
+
+    /* Vary dest size */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memcpy_s(dest, 0, tiny, NUMELMS(tiny));
+    ok(ret == ERANGE, "Copying into a destination of size 0 returned %d, expected ERANGE\n", ret);
+    ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno);
+    okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X');
+
+    /* Replace dest with NULL */
+    errno = 0xdeadbeef;
+    ret = p_memcpy_s(NULL, NUMELMS(dest), tiny, NUMELMS(tiny));
+    ok(ret == EINVAL, "Copying a tiny buffer to a big NULL destination returned %d, expected EINVAL\n", ret);
+    ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno);
+
+    /* Combinations */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memcpy_s(dest, 0, NULL, NUMELMS(tiny));
+    ok(ret == EINVAL, "Copying a NULL buffer into a destination of size 0 returned %d, expected EINVAL\n", ret);
+    ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno);
+    okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X');
+
+    if (p_set_invalid_parameter_handler)
+        ok(p_set_invalid_parameter_handler(NULL) == test_invalid_parameter_handler,
+            "Cannot reset invalid parameter handler\n");
+}
+
+static void test_memmove_s(void)
+{
+    static char dest[8];
+    static const char tiny[] = {'T',0,'I','N','Y',0};
+    static const char big[] = {'a','t','o','o','l','o','n','g','s','t','r','i','n','g',0};
+    int ret;
+    if (!p_memmove_s) {
+        skip("memmove_s not found\n");
+        return;
+    }
+
+    if (p_set_invalid_parameter_handler)
+        ok(p_set_invalid_parameter_handler(test_invalid_parameter_handler) == NULL,
+            "Invalid parameter handler was already set\n");
+
+    /* Normal */
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memmove_s(dest, NUMELMS(dest), tiny, NUMELMS(tiny));
+    ok(ret == 0, "Moving a buffer into a big enough destination returned %d, expected 0\n", ret);
+    okchars(dest, tiny[0], tiny[1], tiny[2], tiny[3], tiny[4], tiny[5], 'X', 'X');
+
+    /* Overlapping */
+    memcpy(dest, big, sizeof(dest));
+    ret = p_memmove_s(dest+1, NUMELMS(dest)-1, dest, NUMELMS(dest)-1);
+    ok(ret == 0, "Moving a buffer up one char returned %d, expected 0\n", ret);
+    okchars(dest, big[0], big[0], big[1], big[2], big[3], big[4], big[5], big[6]);
+
+    /* Vary source size */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memmove_s(dest, NUMELMS(dest), big, NUMELMS(big));
+    ok(ret == ERANGE, "Moving a big buffer to a small destination returned %d, expected ERANGE\n", ret);
+    ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno);
+    okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X');
+
+    /* Replace source with NULL */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memmove_s(dest, NUMELMS(dest), NULL, NUMELMS(tiny));
+    ok(ret == EINVAL, "Moving a NULL source buffer returned %d, expected EINVAL\n", ret);
+    ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno);
+    okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X');
+
+    /* Vary dest size */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memmove_s(dest, 0, tiny, NUMELMS(tiny));
+    ok(ret == ERANGE, "Moving into a destination of size 0 returned %d, expected ERANGE\n", ret);
+    ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno);
+    okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X');
+
+    /* Replace dest with NULL */
+    errno = 0xdeadbeef;
+    ret = p_memmove_s(NULL, NUMELMS(dest), tiny, NUMELMS(tiny));
+    ok(ret == EINVAL, "Moving a tiny buffer to a big NULL destination returned %d, expected EINVAL\n", ret);
+    ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno);
+
+    /* Combinations */
+    errno = 0xdeadbeef;
+    memset(dest, 'X', sizeof(dest));
+    ret = p_memmove_s(dest, 0, NULL, NUMELMS(tiny));
+    ok(ret == EINVAL, "Moving a NULL buffer into a destination of size 0 returned %d, expected EINVAL\n", ret);
+    ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno);
+    okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X');
+
+    if (p_set_invalid_parameter_handler)
+        ok(p_set_invalid_parameter_handler(NULL) == test_invalid_parameter_handler,
+            "Cannot reset invalid parameter handler\n");
+}
+
 static void test_strcat_s(void)
 {
     char dest[8];
@@ -2129,6 +2273,8 @@ START_TEST(string)
         hMsvcrt = GetModuleHandleA("msvcrtd.dll");
     ok(hMsvcrt != 0, "GetModuleHandleA failed\n");
     SET(pmemcpy,"memcpy");
+    p_memcpy_s = (void*)GetProcAddress( hMsvcrt, "memcpy_s" );
+    p_memmove_s = (void*)GetProcAddress( hMsvcrt, "memmove_s" );
     SET(pmemcmp,"memcmp");
     SET(p_mbctype,"_mbctype");
     SET(p__mb_cur_max,"__mb_cur_max");
@@ -2176,6 +2322,8 @@ START_TEST(string)
    /* test _strdup */
     test_strdup();
     test_strcpy_s();
+    test_memcpy_s();
+    test_memmove_s();
     test_strcat_s();
     test__mbsnbcpy_s();
     test_mbcjisjms();