CString and strcpy in VS 2008 and VC++ 6.0

I was faced with an interesting problem recently, which originated from a mix between the CString class and the C-style char* strings.

An ocx (yes, apparently they are not extinct) component written in C++ was receiving a string from a .NET application. After executing a bunch of procedures on this string, it was supposed to return it to the calling .NET application, to be displayed. The ocx’s solution was VS 2008.

The string was at some point going to the component as ‘99’ and it was receiving an initial ‘0’, so the expected result was ‘099’. Instead, we were receiving only ‘09’.

In the component’s code we had a CString object holding the string value and passing it to various methods and functions. Some of these functions were old C functions, and they were working on the string held in the CString object.

What we had could be reduced to this:

         CString test;
         test = "99";
         char* pchar = (char*)test.GetBuffer(0);
         strcpy(pchar, "099");
         test.ReleaseBuffer();
         _tprintf(_T(“%s”), test);

 

The strcpy part was coming from one of the C-style functions, and was supposed to add the extra ‘0’ to the beginning of the string.

GetBuffer() is returning a pointer to the object’s null terminated character buffer. So we had a pointer to a string with 2 characters, and then we copied 3 characters at this address: strcpy(pchar, "099");

 

We were lucky enough not to corrupt any other program variable, but we ran into trouble a little bit later, when we called test.ReleaseBuffer():

void ReleaseBuffer( _In_ int nNewLength = -1 )
                {
                   if( nNewLength == -1 )
                   {
                       int nAlloc = GetData()->nAllocLength;
                       nNewLength = StringLengthN( m_pszData, nAlloc);
                   }
                       SetLength( nNewLength );
                }

 

As you can see, we already have a nAllocLength member variable of CString holding the string’s length. And that is 2, even after we copied the 3-characters string over the buffer. So, we will be calling SetLength(nNewLength) with a value of 2:

void SetLength( _In_ int nLength )
                {
                  if( nLength < 0 || nLength > GetData()->nAllocLength)
                      AtlThrow(E_INVALIDARG);                         

                  GetData()->nDataLength = nLength;
                  m_pszData[nLength] = 0;
                }

So, in the final line of SetLength, the last character of ‘099’ is set to 0, and the string becomes ‘09’.

As the developer of the control confirmed that this problem was not present in the previous versions, I checked this behavior on VC++ 6.0 also.

Here, the definition of ReleaseBuffer is different:

void CString::ReleaseBuffer(int nNewLength)
{
   CopyBeforeWrite(); // just in case GetBuffer was not called

   if (nNewLength == -1)
     nNewLength = lstrlen(m_pchData); // zero terminated

   ASSERT(nNewLength <= GetData()->nAllocLength);
   GetData()->nDataLength = nNewLength;
   m_pchData[nNewLength] = '\0';
}

We don’t have a member variable in CString to hold the string’s length anymore, but we take the length directly from the string’s buffer, with lstrlen(m_pchData). This will allow us to write any string, with no regards to the initial size held in the CString class, and we will still get the length. Then the string will be nicely ended with ‘\0’:            m_pchData[nNewLength] = '\0';

This will make the code ‘work’ in VC++ 6.0 and it will cause the unexpected behavior once we move to newer versions.

To fix this, we need to pass the length of the incoming string to GetBuffer. Instead of

char* pchar = (char*)test.GetBuffer(0);

we will have

char* pchar = (char*)test.GetBuffer(3);

This way, there will be no going over the boundaries of the existing string, and the length will also be properly defined, so ReleaseBuffer and SetLength will not mess with the string anymore.

Comments

• Anonymous
  August 23, 2012
  Any reason not to replace the GetBuffer/strcpy/ReleaseBuffer() by this?  test = "099";
• Anonymous
  August 23, 2012
  The part of the application which was in charge of determining whether the number should receive a '0' in front or not was written in C. So in this particular case it was necessary to work with char*. Of course, things would have been simpler if only CString manipulations had been used.