Buffer too small when copying a string using wcsncpy_s.

Question

This C++ code is kind of lame, but I need to maintain it. I cannot seem to figure out a "buffer too small" problem. I am using Visual Studio 2010. I will come up with minimal code required to reproduce based on the values I see in the debugger. Sorry, I will not have tested the actual snippet itself. Also, since my system clipboard is "busy" while I debug, I cannot just copy and paste, so some error might creep in somewhere, but I will double-check stuff. Trust me, you do not want to see the whole function - it is way too long to make any sense :)

From tchar.h

#define _tcsncpy_s wcsncpy_s

From afxstr.h:

typedef ATL::CStringT< TCHAR, StrTraitMFC_DLL< TCHAR > > CString;

From WinNT.h:

typedef WCHAR TCHAR, *PTCHAR;

Oh, man, these macros never seem to end. I will stop here. Finally, from myfile.cpp:

CString str; // Actually a function parameter with value "07/02/2010"
DWORD nIndex = 10;
DWORD nLast = 0;
LPCTSTR psz = (LPCTSTR) str; // Debugger says that it also gets "07/02/2010".

CString s;
_tcsncpy_s(
    s.GetBuffer((int) (nIndex - nLast + 1)), // I added the " + 1" part hoping to fix a bug, but that changed nothing
    nIndex - nLast,
    psz + nLast,
    (size_t) (nIndex - nLast)
);

With this I hit an assertion, and the debugger opens tcsncpy_s.inl with the following code at the end:

  53    ...
  54    if (available == 0)
  55    {
  56        if (_COUNT == _TRUNCATE)
  57        {
  58            _DEST[_SIZE - 1] = 0;
  59            _RETURN_TRUNCATE;
  60        }
  61        RESET_STRING(_DEST, _SIZE);
=>62        _RETURN_BUFFER_TOO_SMALL(_DEST, _SIZE);
  63    }
  64    _FILL_STRING(_DEST, _SIZE, _SIZE - available + 1);
  65    _RETURN_NO_ERROR;
  66 }
  67
  68

The debugger points at line 62: _RETURN_BUFFER_TOO_SMALL. Unfortunatelly I cannot view the values of things while in the tcsncpy_s.inl. Perhaps an experienced coder could tell me what is going on here? I believe (perhaps incorrectly) that this code is quite old, and not written with Unicode in mind. What is the best way to fix this sticking to the old N crappy guns (No C++0X tricks or other fancy stuff please) - I just want to put a patch on a bullet wound.

Answer 1

The size passed to wcsncpy_s() is the buffer size, not the number of characters the buffer can store. It includes the zero terminator. You need to add 1.

Answer 2

The fourth argument to strncpy_s is the number of characters to copy from the source buffer, and it does not account for the terminating null - i.e., in practice, if the source buffer contains a string that has (nIndex - nLast) or more characters, then (nIndex - nLast) will be copied, and then also a null character will be appended. So the destination buffer must be prepared to accept (nIndex - nLast + 1) characters to also account for that null.

Now your +1 seemingly does that, but you should also reflect it in the second argument to strncpy_s, which tells it how large the buffer is. Change it to (nIndex - nLast + 1), and it should work.

Answer 3

From the docs for wcsncpy_s() (http://msdn.microsoft.com/en-us/library/5dae5d43.aspx):

These functions try to copy the first D characters of strSource to strDest, where D is the lesser of count and the length of strSource. If those D characters will fit within strDest (whose size is given as numberOfElements) and still leave room for a null terminator, then those characters are copied and a terminating null is appended; otherwise, strDest[0] is set to the null character and the invalid parameter handler is invoked

So, the counts must be specified in characters (elements), not bytes, which you're already doing, And the destination buffer size must account for the null terminator character, which you're making room for in the buffer, but not telling the wcsncpy_s() (known as _tcsncpy() here) about:

_tcsncpy_s(
    s.GetBuffer((int) (nIndex - nLast + 1)), // I added the " + 1" part hoping to fix a bug, but that changed nothing
    nIndex - nLast + 1,
    psz + nLast,
    (size_t) (nIndex - nLast)
);