Strange Output with WM_SETTEXT

[Dyndrilliac]

Here's the Test Application output:

Please enter the Window Caption you wish to change: Calculator

Please enter the new Window Caption for "Calculator": Change

Success! The Window's Caption has been changed successfully.
The Window Caption is: Change
Press any key to continue

Here's the code:

Code Select Expand

#include <stdio.h>
#include <conio.h>
#include <windows.h>

int main() {
    char *szNewCaption = new char[127];
    char *szOldCaption = new char[127];
    char *szTextBuffer = new char[127];

    printf("Please enter the Window Caption you wish to change: ");
    scanf("%s", szOldCaption);

    HWND My_hWnd = FindWindow(NULL, szOldCaption);

    while (My_hWnd == NULL) {
        printf("\nThe Window Caption you have specified does not exist, or is a child window.");
        printf("\nPlease enter a valid Window Caption: ");
        scanf("%s", szOldCaption);
    }

    printf("\nPlease enter the new Window Caption for \"%s\": ", szOldCaption);
    scanf("%s", szNewCaption);

    if (SendMessage(My_hWnd, WM_SETTEXT, NULL, (DWORD)&szNewCaption) == true) {
        printf("\nSuccess! The Window's Caption has been changed successfully.\n");
        if (SendMessage(My_hWnd, WM_GETTEXT, 127, (DWORD)&szTextBuffer) != 0) {
            printf("The Window Caption is: %s\n", szTextBuffer);
        } else {
            printf("An error has occurred. The Window Caption cannot be read.\n");
        }
    } else {
        printf("\nFailure! The Window's Caption has not been changed successfully.\n");
    }

    getch();
    return 0;
}

Everything runs fine, perfectly; except when I call SendMessage() with WM_SETTEXT the Window's Caption doesn't change to what I specify, even though WM_GETTEXT and the error handlers I set up say all-clear. The Caption changes, but only to extended ASCII symbols (Something along the lines of: "0☼x").

Is there a way I can fix this?

[Arta]

Your lparam is supposed to be a pointer to the string, but you're passing a pointer to the pointer that points to your string:

Code Select Expand


SendMessage(My_hWnd, WM_SETTEXT, NULL, (DWORD)&szNewCaption)


In other words, you're passing a pointer to szNewCaption -- which is already a pointer to the string. You don't need the address-of operator (&).

As to why the correct string is returned by WM_GETTEXT, I'm not sure. Not really in the mood to put my thinking cap on

Edit:

Ahh! I just saw it. It's because you're setting the window caption to the value of the pointer to the string (because you're passing a pointer to a pointer). When you get the window caption, the pointer value is placed in szTextBuffer: in other words, szTextBuffer ends up pointing to the same string. Add this check before getch() and you'll see what I mean:

Code Select Expand


if(szTextBuffer == szNewCaption)
{
printf("Boo.");
}


Also, you've made the same mistake with WM_GETTEXT as you have with WM_SETTEXT: you don't need to use & there either. You should be freeing that memory you're allocating at the beginning as well:

Code Select Expand


delete [] szNewCpation;
delete [] szOldCaption;
delete [] szTextBuffer;


Strictly speaking, this is redundant, because the OS will free your memory when your program terminates. This, however, is considered by pretty much everyone to be very bad practice: you should get into the habit of placing a corresponding delete for each allocation in your program, as a strategy for avoiding memory leaks.

[Dyndrilliac]

Hm. Ok, I changed the block calling SendMessage() to this:

Code Select Expand

    if (SendMessage(My_hWnd, WM_SETTEXT, NULL, (DWORD)&szNewCaption) == true) {
        printf("\nSuccess! The Window's Caption has been changed successfully.\n");
        if (SendMessage(My_hWnd, WM_GETTEXT, 127, (DWORD)szTextBuffer) != 0) {
            printf("The Window Caption is: %s\n", szTextBuffer);
        } else {
            printf("An error has occurred. The Window Caption cannot be read.\n");
        }
    } else {
        printf("\nFailure! The Window's Caption has not been changed successfully.\n");
    }

But now, I get this output:

Please enter the Window Caption you wish to change: Calculator

Please enter the new Window Caption for "Calculator": Change

Success! The Window's Caption has been changed successfully.
The Window Caption is: 0▼╫
Press any key to continue

So now it still doesn't change the Window Caption properly, and GETTEXT no longer returns the correct text.

[Arta]

Neither of your calls to SendMessage should be using the address-of operator. Your sz variables are already pointers.

[Dyndrilliac]

Thanks, it works perfectly now

And I'll be sure to remember the advice on delete, too.

[Kp]

A few stylistic notes: scanf should be treated like gets() : too dangerous to use in any real application.  You could add a constraint to limit the amount of data scanf returns, but scanf is so picky about input format that I rarely find a use for it.  The problem with your usage here is that there's nothing to stop the user from entering hundreds (or thousands) of characters, and trashing your heap in the process.

Second, for a temporary with function-length lifespan and small size, as your sz variables are, it's a bit strange looking to allocate them on the heap.  Perhaps you're still recovering from a brain-damaged language like Java, which requires this?   Anyway, it's quite ok to say:

Code Select Expand

char sz1[127], sz2[127];and then use them in the same fashion as you used your other pointers.  This will run faster (no dynamic memory allocation) and spare you needing to free the variables when you're done.  Of course, it's unsuitable if you want to return the result to the caller, or you expect to allocate a really large chunk (as in, many megabytes).  In such a situation, your approach is better.

[Dyndrilliac]

Thanks for the tips KP.

If scanf() isn't worth the trouble to use, what should I use for user input? And yes, I took a Java class recently, which rubbed off on me.

[Kp]

scanf is OK to use, if you're cautious about its dangers.  For instance, %s scans in only one word, but that word can be arbitrarily long.  The user could feed you a million As, and scanf would copy them to your output buffer.  For input where you know it follows some format (e.g. scanning back in a config file your program generated), it's perfectly OK to use scanf (more precisely, sscanf or fscanf).  Some people are fond of using strtok(3), but I've never liked it.  I usually end up doing string parsing "by hand" (i.e. walking the buffer validating/converting input as I go), since I like to be able to do graceful recoveries and informative errors.  Check out fgets(3), scanf(3), strtoul(3), strchr(3), strtok(3) for more info on how the different functions operate.  Although, I'm not the best person to ask about user input, since I primarily write code that doesn't deal (directly) with user input.   Most of my stuff gets buffered through other programs or GUIs first.

[Yoni]

scanf is fine if you never use %s.
%Ns for a positive integer N is standard usage and is acceptable. Example:

Code Select Expand

char buf[64];
scanf("%64s", buf);

strtok is dangerous because it relies on a null terminator being present. If by some mistake it isn't, you're in trouble. Be careful with it.

[Blaze]

Is there anything wrong with using gets() ?

[Yoni]

Is there anything wrong with using gets() ?

Yes. gets is the classic example of functions that should never be used. This is documented in the manpage, too.
Instead, use fgets(buf, size, stdin).