12

Trying to append text to an edit control inside a dialog box. I can't get _tcscat_s to append correctly. It crashes and says something about the buffer being too small or something about a null terminated string.

int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nCmdShow )
{
    return DialogBox( hInstance, MAKEINTRESOURCE( IDD_MAIN ), NULL, DlgProc );
}

BOOL CALLBACK DlgProc( HWND hwnd, UINT Message, WPARAM wParam, LPARAM lParam ) 
{
    switch( Message )
    {
        case WM_INITDIALOG:
            OpenAndReadFile( hwnd );
            return TRUE;
        case WM_COMMAND:
            switch( LOWORD( wParam ) )
            {
                case IDSTART:
                    EndDialog( hwnd, IDSTART );
                    break;
                case IDQUIT:
                    EndDialog( hwnd, IDQUIT );
                    break;
            }
            break;
        case WM_CLOSE:
            EndDialog( hwnd, 0 );
            break;
        default:
            return FALSE;
    }
    return TRUE;
}

BOOL OpenAndReadFile( const HWND &hwnd ) 
{   
    // Open the file

    HANDLE hFile;
    hFile = CreateFile( TEXT( "sites.txt" ),    // file to open
                        GENERIC_READ,           // open for reading
                        FILE_SHARE_READ,        // share for reading
                        NULL,                   // default security
                        OPEN_EXISTING,          // existing file only
                        FILE_ATTRIBUTE_NORMAL,  // normal file
                        NULL );                 // no attr. template

    if ( hFile == INVALID_HANDLE_VALUE )
    {
        SetDlgItemText( hwnd, IDC_OUTPUT, TEXT( "Error: File could not be opened\r\n" ) );
        return FALSE;
    }
    else
        SetDlgItemText( hwnd, IDC_OUTPUT, TEXT( "sites.txt opened\r\n" ) );

    AppendText( hwnd, TEXT("TEXT") );

    // Read data from file

    const DWORD BUFFERSIZE = GetFileSize( hFile, NULL );
    char *ReadBuffer = new char [BUFFERSIZE]();
    DWORD dwBytesRead = 0;

    // read one character less than the buffer size to save room for the
    // terminate NULL character.
    //if ( FALSE == ReadFile( hFile, ReadBuffer, BUFFERSIZE - 1, &dwBytesRead, NULL ) )
    {

    }

    return TRUE;
}

void AppendText( const HWND &hwnd, TCHAR *newText )
{
    // get size to determine buffer size
    int outLength = GetWindowTextLength( GetDlgItem( hwnd, IDC_OUTPUT ) );

    // create buffer to hold current text in edit control
    TCHAR * buf = ( TCHAR * ) GlobalAlloc( GPTR, outLength + 1 );

    // get existing text from edit control and put into buffer
    GetDlgItemText( hwnd, IDC_OUTPUT, buf, outLength + 1 );

    // append the newText to the buffer
    _tcscat_s( buf, outLength + 1, newText );

    // Set the text in the dialog
    SetDlgItemText( hwnd, IDC_OUTPUT, buf );
}
ShrimpCrackers
  • 4,388
  • 17
  • 50
  • 76
  • I have been assuming that _tcscat_s would add the space necessary when doing a concatenation. Is this wrong? If so, would I need to make extra space for the newText during GlobalAlloc? – ShrimpCrackers May 02 '13 at 01:35
  • Why use it at all? To use the same algorithm, just load the text into a `std::string`, append to it, and save it back. In C++11 at least, the data is guaranteed to be contiguous. If not using C++11, `std::vector` works almost just as well to load the text. Alternatively (and probably better), use [this technique](http://stackoverflow.com/a/12538062/962089). No managing memory for it. No worries about appending. Just as easy to use. – chris May 02 '13 at 01:39
  • @ShrimpCrackers: `_tcscat_s()` DOES NOT grow the buffer if the appended text were to exceed the bounds of the buffer. `_tcscat_s()` has a parameter to specify the buffer size so it will truncate the concatenated text if it becomes too long. As such, you need to allocate the buffer to the required final size before you start putting text into it. – Remy Lebeau May 02 '13 at 02:08
  • possible duplicate of [How to append text to a TextBox?](http://stackoverflow.com/questions/12537456/how-to-append-text-to-a-textbox) – Remy Lebeau May 02 '13 at 02:09

2 Answers2

26

GetWindowTextLength() returns the number of TCHAR elements in the text, but GlobalAlloc() expects a byte count instead. If you are compiling for Unicode, TCHAR is 2 bytes, not 1 byte, but you are not taking that into account. You are also not allocating the buffer large enough to hold both the existing text and the new text being appended. You are also leaking the memory that you allocate.

Try this:

void AppendText( const HWND &hwnd, TCHAR *newText )
{
    // get edit control from dialog
    HWND hwndOutput = GetDlgItem( hwnd, IDC_OUTPUT );

    // get new length to determine buffer size
    int outLength = GetWindowTextLength( hwndOutput ) + lstrlen(newText) + 1;

    // create buffer to hold current and new text
    TCHAR * buf = ( TCHAR * ) GlobalAlloc( GPTR, outLength * sizeof(TCHAR) );
    if (!buf) return;

    // get existing text from edit control and put into buffer
    GetWindowText( hwndOutput, buf, outLength );

    // append the newText to the buffer
    _tcscat_s( buf, outLength, newText );

    // Set the text in the edit control
    SetWindowText( hwndOutput, buf );

    // free the buffer
    GlobalFree( buf );
}

Alternatively:

#include <vector>

void AppendText( const HWND &hwnd, TCHAR *newText )
{
    // get edit control from dialog
    HWND hwndOutput = GetDlgItem( hwnd, IDC_OUTPUT );

    // get new length to determine buffer size
    int outLength = GetWindowTextLength( hwndOutput ) + lstrlen(newText) + 1;

    // create buffer to hold current and new text
    std::vector<TCHAR> buf( outLength );
    TCHAR *pbuf = &buf[0];

    // get existing text from edit control and put into buffer
    GetWindowText( hwndOutput, pbuf, outLength );

    // append the newText to the buffer
    _tcscat_s( pbuf, outLength, newText );

    // Set the text in the edit control
    SetWindowText( hwndOutput, pbuf );
}

With that said, getting the window's current text into memory, appending to it, and then replacing the window's text is a very inefficient way to append text to an edit control. Use the EM_REPLACESEL message instead:

void AppendText( const HWND &hwnd, TCHAR *newText )
{
    // get edit control from dialog
    HWND hwndOutput = GetDlgItem( hwnd, IDC_OUTPUT );

    // get the current selection
    DWORD StartPos, EndPos;
    SendMessage( hwndOutput, EM_GETSEL, reinterpret_cast<WPARAM>(&StartPos), reinterpret_cast<WPARAM>(&EndPos) );

    // move the caret to the end of the text
    int outLength = GetWindowTextLength( hwndOutput );
    SendMessage( hwndOutput, EM_SETSEL, outLength, outLength );

    // insert the text at the new caret position
    SendMessage( hwndOutput, EM_REPLACESEL, TRUE, reinterpret_cast<LPARAM>(newText) );

    // restore the previous selection
    SendMessage( hwndOutput, EM_SETSEL, StartPos, EndPos );
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Inefficient and just adds more likelihood of screwing something up, which is pretty evident in all of those problems you pointed out. – chris May 02 '13 at 01:53
  • Thanks, Remy. Works great. Being new to win32 api, I have three questions about that last example. (1) Is the hwndOutput assignment necessary? I thought I was passing in handle to the dialog control as a function parameter. Is the hwnd from the DlgProc() function a handle to something else? (2) Why the reinterpret_cast on WPARAM and LPARAM? (3) Why does outLength have to be sent to both WPARAM and LPARAM parameters? – ShrimpCrackers May 02 '13 at 02:10
  • 2
    1) the `EM_...` messages go to the edit control directly so it makes sense to grab its HWND once and use it multiple times. The DlgProc() HWND is the parent dialog. 2) `reinterpret_cast` is C++'s type-safe, compiletime-validated way to type cast a pointer to an integer and vice versa (amongst other things). 3) [read the documentation](http://msdn.microsoft.com/en-us/library/windows/desktop/bb761661.aspx). 4) you have a memory leak in `OpenAndReadFile()`. – Remy Lebeau May 02 '13 at 04:30
  • @RemyLebeau Does the pointer `newText` need to remain available even after this `SendMessage` function returns? For example, if you call this in a function and you pass in a stack allocated string and then return from that function, the pointer has been freed, therefore could corruption happen? The documentation on MSDN doesn't mention anything about the lifetime of this pointer. – Code Doggo Oct 03 '22 at 02:05
  • 1
    @CodeDoggo the Edit control does not take ownership of the pointer, it makes a copy of the string data being pointed at, so no, the pointer does not need to persist after `SendMessage()` returns – Remy Lebeau Oct 03 '22 at 02:13
  • @RemyLebeau Thanks for clearing that up! I do have one last question. After you append 32kb of characters, the edit control is filled up completely. I understand you can set an even larger max limit via `EM_SETLIMITTEXT`, but I am wondering if the oldest set of characters can be deleted in order to accommodate the new text? Can this be done without having to re-draw all of the text again? For example, in a console window, new output is constantly logged to the GUI window, but the control buffer eventually removes/deletes the oldest lines without having to re-draw all the text that remains. – Code Doggo Oct 03 '22 at 02:20
  • @CodeDoggo just as `EM_REPLACESEL` can add text, it can also remove text. Simply use `EM_SETSEL` to select the desired old text and then send `EM_REPLACESEL` with `lParam` set to `""` to erase the text. – Remy Lebeau Oct 03 '22 at 04:23
18

http://support.microsoft.com/kb/109550

Is your answer:

   string buffer = "append this!"
   HWND hEdit = GetDlgItem (hDlg, ID_EDIT);
   int index = GetWindowTextLength (hEdit);
   SetFocus (hEdit); // set focus
   SendMessageA(hEdit, EM_SETSEL, (WPARAM)index, (LPARAM)index); // set selection - end of text
   SendMessageA(hEdit, EM_REPLACESEL, 0, (LPARAM)buffer.c_str()); // append!
fider
  • 1,976
  • 26
  • 29