Casting string type with GetDlgItemText() for use as string buffer in C++

Question

I am stumped by the behaviour of the following in my Win32 (ANSI) function: (Multi-Byte Character Set NOT UNICODE)

void sOut( HWND hwnd, string sText ) // Add new text to EDIT Control
{ 
 int len;
 string sBuf, sDisplay;

  len = GetWindowTextLength( GetDlgItem(hwnd, IDC_EDIT_RESULTS) ); 
  if(len > 0)
  {
   // HERE:
   sBuf.resize(len+1, 0); // Create a string big enough for the data
   GetDlgItemText( hwnd, IDC_EDIT_RESULTS, (LPSTR)sBuf.data(), len+1 );
  } // MessageBox(hwnd, (LPSTR)sBuf.c_str(), "Debug", MB_OK);

  sDisplay = sBuf + sText;
  sDisplay = sDisplay + "\n\0"; // terminate the string
  SetDlgItemText( hwnd, IDC_EDIT_RESULTS, (LPSTR)sDisplay.c_str() );
}

This should append text to the control with each call.

Instead, all string concatenation fails after the call to GetDlgItemText(), I am assuing because of the typecast?

I have used three string variables to make it really obvious. If sBuf is affected then sDisplay should not be affected.

(also why is len 1 char less than the length in the buffer?)

GetDlgItemText() corretly returns the content of the EDIT control, and SetDlgItemText() will correctly set any text in sDisplay, but the concatenation in between is just not happening.

Is this a "hidden feature" of the string class?

Added:

Yes it looks like the problem is a terminating NUL in the middle. Now I understand why the len +1. The function ensures the last char is a NUL.

Using sBuf.resize(len); will chop it off and all is good.

Thanks for your help

Added:

Charles,

Leaving aside the quirky return length of this particular function, and talking about using a string as a buffer:

The standard describes the return value of basic_string::data() to be a pointer to an array whose members equal the elements of the string itself.

Thats precisely whats needed isn't it?

Further, it requires that the program must not alter any of the values of that array.

As I understand it that is going to change along with the guarantee that all bytes are contiguous. I forget where I read a long article on this, but MS already implements this it asserted.

What I don't like about using a vector, is that the bytes are copied twice before I can return them. Once into the vector and again into the string. I also need to instantiate a vector object and a string object. That is a lot of overhead. If there were some string friendly of working with vectors (or CStrings) without resorting to old C functions or sopying characters one by one, I would use them. The string is very syntax friendly in that way.

Answer 1

The data() function on a std::string returns a const char*. You are not allowed to right into the buffer returned by it, it may be a duplicated buffer.

What you could do instead is to used a std::vector<char> as a temporary buffer.

E.g. (untested)

std::vector<char> sBuf( len + 1 );
GetDlgItemText( /* ... */, &sBuf[0], len + 1 );

std::string newText( &sBuf[0] );
newText += sText;

Also, the string you pass to SetDlgItemText should be \0 terminated so you should used c_str() not data() for this.

SetDlgItemText( /* ... */, newText.c_str() );

Edit:

OK, I've just checked the contract for GetWindowTextLength and GetDlgItemText. Check my edits above. Both will include the space for a null terminator so you need to chop it off the end of your string otherwise concatenation of the two strings will include a null terminator in the middle of the string and the SetDlgItemText call will only use the first part of the string.

There is a further complication in that GetWindowTextLength isn't guaranteed to be accurate, it only guarantees to return a number big enough for a program to create a buffer for storing the result. It is extremely unlikely that this will actually affect a dialog box item owned by the calling code but in other situations the actual text may be shorter than the returned length. For this reason you should search for the first \0 in the returned text in any case.

I've opted to just use the std::string constructor that takes a const char* so that it finds the first \0 correctly.

The standard describes the return value of basic_string::data() to be a pointer to an array whose members equal the elements of the string itself. Further, it requires that the program must not alter any of the values of that array. This means that the return value of data() may or may not be a copy of the string's internal representation and even if it isn't a copy you still aren't allowed to write to it.

Answer 2

In this context if I were you I would probably use CString instead.

string sDisplay
CString sBuf;

GetDlgItemText( hwnd, IDC_EDIT_RESULTS, (LPSTR)sBuf.GetBuffer(len+1), len+1 );

sBuf.Release();

sDisplay = sBuf.GetBuffer() + sText;

IMHO when you are dealing with Win32 using CString is more convenient than using std::string (wstring).

Answer 3

I am far away from the win32 api and their string nightmare, but there is something in the code that you can check. Standard C++ strings do not need to be null terminated and nulls can happen anywhere within the string. I won't comment on the fact that you are casting away constantness with your C-style cast, which is a problem on its own, but rather on the strange effect you are

When you initially create the string you allocate extra space for the null (and initialize all elements to '\0') and then you copy the elements. At that point your string is len+1 in size and the last element is a null. After that you append some other string, and what you get is a string that will still have a null character at position len. When you retrieve the data with either data() (does not guarantee null termination!) or c_str() the returned buffer will still have the null character at len position. If that is passed to a function that stops on null (takes a C style string), then even if the string is complete, the function will just process the first len characters and forget about the rest.

#include <string>
#include <cstdio>
#include <iostream>
int main()
{
   const char hi[] = "Hello, ";
   const char all[] = "world!";
   std::string result;
   result.resize( sizeof(hi), 0 );
   // simulate GetDlgItemText call
   std::copy( hi, hi+sizeof(hi), const_cast<char*>(result.data()) ); // this is what your C-style cast is probably doing
   // append
   result.append( all );

   std::cout << "size: " << result.size() // 14
      << ", contents" << result // "Hello, \0world!" - dump to a file and edit with a binary editor
      << std::endl;
   std::printf( "%s\n", result.c_str() ); // "Hello, "
}

As you can see, printf expects a C-style string and will stop when the first null character is found, so that it can seem as if the append operation never took place. On the other hand, c++ streams do work properly with std::string and will dump the whole content, checking that the strings were actually appended.

A patch to your append operation disappearing would be removing the '\0' from the initial string (reserve only len space in the string). But that is not really a good solution, you should never use const_cast (there are really few places where it can be required and this is not one of them), the fact that you don't see it is even worse: using C style casts is making your code look nicer than it is.

You have commented on another answer that you do not want to add std::vector (which would provide with a correct solution as &v[0] is a proper mutable pointer into the buffer), of course, not adding the extra space for the '\0'. Consider that this is part of an implementation file, and the fact that you use or not std::vector will not extend beyond this single compilation unit. Since you are already using some STL features, you are not adding any extra requirement to your system. So to me that would be the way to go. The solution provided by Charles Bailey should work provided that you remove the extra null character.

Answer 4

This is NOT an answer. I have added it here as an answer only so that I can use formatting in a long going discussion about const_cast.

This is an example where using const_cast can break a running application:

#include <iostream>
#include <map>
typedef std::map<int,int> map_type;
void dump( map_type const & m ); // implemented somewhere else for concision
int main() {
   map_type m;
   m[1] = 10;
   m[2] = 20;
   m[3] = 30;
   map_type::iterator it = m.find(2);
   const_cast<int&>(it->first) = 10;
   // At this point the order invariant of the container is broken:
   dump(); // (1,10),(10,20),(3,30) !!! unordered by key!!!!
   // This happens with g++-4.0.1 in MacOSX 10.5
   if ( m.find(3) == m.end() ) std::cout << "key 3 not found!!!" << std::endl;
}

That is the danger of using const_cast. You can get away in some situations, but in others it will bite back, and probably hard. Try to debug in thousands of lines where the element with key 3 was removed from the container. And good luck in your search, for it was never removed.