3

I have a problem with the wcscpy_s function. After wcscpy_s returns the parameter (stringOneand stringTwo) of my function are not readable. Here is simple demo to show the problem.

void testFunc(LPCWSTR stringOne, LPCWSTR stringTwo) {

    wchar_t* defaultVal = L"Default";

    wchar_t tmp[100];

    int lenBefore = wcslen(stringOne); // Works

    auto result = wcscpy_s(tmp, sizeof(tmp), defaultVal);

    int len = wcslen(tmp);

    int len2 = wcslen(stringOne); // Throws Exception Access violation
}


int main() {
    testFunc(L"Test", L"Test");
}
Mr.C64
  • 41,637
  • 14
  • 86
  • 162
Kevin
  • 785
  • 2
  • 10
  • 32
  • 1
    One problem is that `sizeof(tmp)` gives you the size in *bytes*, but it should be the number of *elements* in the array. See e.g. [this `wcscpy_s` reference](https://msdn.microsoft.com/en-us/library/td1esda9.aspx). – Some programmer dude Oct 10 '17 at 08:59

2 Answers2

3

The documentgation of wcscpy_s states that the debug version of this function fills the destination buffer with the special value 0xFE.

When you call wcscpy_s(tmp, sizeof(tmp), defaultVal); you pass the size of the tmp buffer, but wcscpy_s wants the length in number of characters. Therefore the length you pass to wcscpy_s is twice as long as it should be, and as the tmp buffer is overwritten by 0xfe you get a buffer overflow and undefined behaviour, even if the length if the source string (L"Default";) is small.

So use _countof(tmp) instead of _sizeof(tmp).

This said, I suggest you learn how to use the Visual Studio debugger.

Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
2

As already explained in Michael Walz's answer, you have a buffer overflow caused by passing an incorrect buffer size.

In addition to his suggestion of using _countof(tmp) instead of sizeof(tmp), I'd like to add that in C++ there's a convenient overload of wcscpy_s() that automatically deduces the correct buffer size:

template <size_t size>  
errno_t wcscpy_s(  
   wchar_t (&strDestination)[size],  
   const wchar_t *strSource   
); // C++ only

Basically, you can write simpler code like this, that will just work:

wchar_t tmp[100];

// Use the C++-only template overload of wcscpy_s
// that automatically deduces the destination buffer size
auto result = wcscpy_s(tmp, defaultVal);

If you use this overload, you are immune to those sizeof/_countof mismatch kind of bugs.

Note that this C++ overload works only if you have a static buffer like your wchar_t tmp[100], as the C++ compiler must be able to figure out the buffer size at compile-time. On the other hand, when you have pointers to dynamically-allocated buffers, you have to pass the correct buffer size explicitly.

Mr.C64
  • 41,637
  • 14
  • 86
  • 162