[winapi] edit control is lying to me...

ubergeek · #1 13-May-2005, 20:16

code says a thousand words:

CPP / C++ / C Code:

bool saveFile(HWND hwnd, char *filename)
{
    char *realfilename = filename;
    if (realfilename == NULL)
    {
        char selectedfilename[MAX_PATH+1] = "";
        
        OPENFILENAME sfn = { 0 };
        sfn.lStructSize = sizeof(OPENFILENAME);
        sfn.hwndOwner = hwnd;
        sfn.lpstrFilter = "Text Files\0*.txt\0GeekEdit Files\0*.gke\0All Files\0*.*\0";
        sfn.lpstrFile = selectedfilename;
        sfn.nMaxFile = MAX_PATH;
        sfn.lpstrTitle = "Geekedit: Save File";
        sfn.Flags = OFN_OVERWRITEPROMPT;
        sfn.lpstrDefExt = "txt";
        
        if (!GetSaveFileName(&sfn))
        {
            errmsg(hwnd, "Save dialog box failed.");
            return false;
        }
        realfilename = selectedfilename;
    }
    
    HWND hEdit = GetDlgItem(hwnd, IDC_EDIT);
    if (hEdit == NULL)
    {
        errmsg(hwnd, "Could not find edit control.");
        return false;
    }
    
    int filetextlen = GetWindowTextLength(hEdit);
    if (filetextlen == 0)
    {
        errmsg(hwnd, "No text in edit control or could not ascertain length of text in edit control.");
        return false;
    }
    char *filetext = new char[filetextlen+1];
    if (filetext == NULL)
    {
        errmsg(hwnd, "Out of memory.");
        return false;
    }
    filetext = "";
    GetWindowText(hEdit, filetext, filetextlen);
    dbgmsg(hwnd, filetext);
    if (filetext == "")
    {
        errmsg(hwnd, "No text in edit control or failed to retrieve text from edit control because:");
        DWORD err = GetLastError();
        char *errstr = NULL;
        FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, NULL, err, 0, errstr, 0, NULL);
        dbgmsg(hwnd, errstr);
        LocalFree(errstr);
        return false;
    }
    
    ofstream file(realfilename);
    if (!file.is_open())
    {
        errmsg(hwnd, "Could not open file for output.");
        return false;
    }
    file << filetext << flush;
    file.close();
    
    return true;
}

this is a function in my notepad-like windows application. It is passed the HWND of the program's main window and a filename (although if the filename passed is NULL it prompts the user for one). My problem is that the call to GetWindowText doesn't work. Even if I type something into the edit control, I get an empty string (""). This seems rather odd to me. What am I doing wrong?
EDIT: forgot to add, GetLastError tells me that nothing went wrong (zero error code and empty string returned by FormatMessage).
also, dbgmsg and errmsg are defined as:

CPP / C++ / C Code:

inline int dbgmsg(HWND hwnd, char *msg) { return MessageBox(hwnd, msg, "Geekedit: Debugging Message", MB_OK | MB_ICONINFORMATION); }
inline int errmsg(HWND hwnd, char *msg) { return MessageBox(hwnd, msg, "Geekedit: Error", MB_OK | MB_ICONERROR); }
.

Dr. Evil · #2 14-May-2005, 02:19

I'm afraid I can't really find what's wrong in your code, unless you should use GlobalAlloc() instead of new, but here's my saving function if that helps at all:

CPP / C++ / C Code:

int SaveTextToFile(HWND winhandle, LPSTR filename)
{
	HWND edithandle;
	HANDLE out;
	LPSTR buffer;
	DWORD size, writ;
	int ret;
	
	ret = 1;
	edithandle = GetDlgItem(winhandle, ID_MAIN_EDIT);
	out = CreateFile(filename, 
		GENERIC_WRITE, FILE_SHARE_WRITE, 
		(LPSECURITY_ATTRIBUTES)NULL, CREATE_ALWAYS, 
		0, (HANDLE)NULL);
	if(out != INVALID_HANDLE_VALUE)
	{
		size = GetWindowTextLength(edithandle);
		if(size > 0)
		{
			buffer = GlobalAlloc(GPTR, size+1);
			if(buffer != NULL)
			{
				if(GetWindowText(edithandle, buffer, size+1))
				{
					if(WriteFile(out, buffer, size, &writ, NULL)) ret = 0;
				}
				GlobalFree(buffer);
			}
		}
		else ret = 0;
		CloseHandle(out);
	}
	
	return ret;
}

Kinda pointless error handling there, I know, but there's probably some sort of reason somewhere buried in that mass of files and code.

davekw7x · #3 14-May-2005, 14:17

Quote:

Originally Posted by ubergeek

code says a thousand words:

CPP / C++ / C Code:


    char *filetext = new char[filetextlen+1];
    if (filetext == NULL)
    {
        errmsg(hwnd, "Out of memory.");
        return false;
    }
    filetext = "";
    GetWindowText(hEdit, filetext, filetextlen);
    dbgmsg(hwnd, filetext);
    if (filetext == "")
    {

Here's the sequence:

You declare filetext to be a pointer to char and set its value to point to a dynamically allocated block of chars.

Then you set the value of filetext to point to the address of the string literal "" (some place in program space that's not available to you for writing).

So, first problem: You can never delete the dynamically allocated memory since you have lost its address (that's known as a memory leak).

Then, you call a function (it doesn't really matter what function) and pass the value of the pointer to the function. I don't care what the function tries to do with the pointer, the fact is that the value of the pointer in the calling function is not changed. (If the function does try to write something to that address, the behavior is undefined, since addresses of string literals are not supposed to be used for writing. The progam might crash --- or not.)

In any case, when your comparison statement tests to see if the value if filetext is still equal to the same value that it was before the function call: it is.

The fix for this particular problem:

In general you could change this

CPP / C++ / C Code:


    filetext = "";

to the following:

CPP / C++ / C Code:


    strcpy(filetext, "");

or maybe to this:

CPP / C++ / C Code:

   *filetext = '\0';

Then, after the function call, change this

CPP / C++ / C Code:


    if (filetext == "")

to something else.

Maybe this:

CPP / C++ / C Code:

  if (strlen(filetext) == 0) /* see if string is empty */

or this

CPP / C++ / C Code:

  if (*filext == '\0') /* see if string is empty */

or something similar.

Don't forget to delete [] filetext sometime before your program terminates.

Note that for this particular function, GetWindowText(), you don't have to initiliaze the string to be empty before the call; just allocate memory, call the function and test the length of the string after the function returns. In other words, the strcpy() that I show above is not really necessary. (It doesn't do any particular harm, but it clutters up the code with unnecessary activity.)

Also, the function returns the length of the string, so you only need to test its return value. That is, instead of testing the return string to see if it's empty, you could do something like this:

CPP / C++ / C Code:


    if(!GetWindowText(...,...,...)) /* see if return value is equal to zero */

Regards,

Dave

ubergeek · #4 14-May-2005, 15:35

thanks Dave for catching that error. I thought that a dynamically allocated array could be treated like a regular char arry, I guess I was wrong. Here is the working saveFile function and its two helper functions, in case anyone is interested:

CPP / C++ / C Code:

bool saveFile(HWND hwnd, char *filename) //hwnd: window the function can use to display dialog boxes. In addition, hwnd must have a child window with the identifier IDC_EDIT that is the edit control from which to save the text. filename: filename to save to, or NULL if the function should use a dialog box to get a new filename
{
    char *realfilename = filename; //declare new filename pointer so that following if block can, if necessary, get a new filename without modifying the original
    if (realfilename == NULL)
    {
        char selectedfilename[MAX_PATH+1] = "";
        
        OPENFILENAME sfn = { 0 };
        sfn.lStructSize = sizeof(OPENFILENAME);
        sfn.hwndOwner = hwnd;
        sfn.lpstrFilter = "Text Files\0*.txt\0GeekEdit Files\0*.gke\0All Files\0*.*\0";
        sfn.lpstrFile = selectedfilename;
        sfn.nMaxFile = MAX_PATH;
        sfn.lpstrTitle = "Geekedit: Save File";
        sfn.Flags = OFN_OVERWRITEPROMPT;
        sfn.lpstrDefExt = "txt";
        
        if (!GetSaveFileName(&sfn))
        {
            errmsg(hwnd, "Save dialog box failed.");
            return false;
        }
        realfilename = selectedfilename;
    }
    
    HWND hEdit = GetDlgItem(hwnd, IDC_EDIT);
    if (hEdit == NULL)
    {
        errmsg(hwnd, "Could not find edit control.");
        return false;
    }
    
    int filetextlen = GetWindowTextLength(hEdit);
    if (filetextlen == 0)
    {
        errmsg(hwnd, "No text in edit control or could not ascertain length of text in edit control.");
        return false;
    }
    char *filetext = new char[filetextlen+1];
    if (filetext == NULL)
    {
        errmsg(hwnd, "Out of memory.");
        return false;
    }
    if (!GetWindowText(hEdit, filetext, filetextlen))
    {
        errmsg(hwnd, "No text in edit control or failed to retrieve text from edit control.");
        return false;
    }
    
    char *fixedfilename = new char[strSlashesLen(realfilename)+1]; //declare new string to hold the filename with all \\s changed to \\\\s
    strSlashes(realfilename, fixedfilename); //copy realfilename to fixedfilename, with all instances of \\ changed to \\\\
    
    ofstream file(fixedfilename);
    if (!file.is_open())
    {
        errmsg(hwnd, "Could not open file for output.");
        return false;
    }
    file << filetext << flush; //output text and flush the buffer.
    file.close();
    
    delete[] filetext;     //memory leaks are bad
    delete[] fixedfilename;
    
    return true;
}

void strSlashes(char *str, char *newstr) //copies str to newstr, while replacing all instances of \\ with \\\\. newstr must be allocated to enough space to hold all of str plus an extra char for each \\ (it is suggested to call strSlashesLen to get this value)
{
    int newi = 0; //i would declare them both in the initialization block of the for loop but that doesn't work for some reason. also, I need newi later to ascertain the length of newstr
    for (int oldi = 0; str[oldi]; ++oldi, ++newi)
    {
        newstr[newi] = str[oldi]; //this is what strcpy does
        if (str[oldi] == '\\') //contrary to appearances, this is checking for \\ (because of escape sequences)
        {
            ++newi;
            newstr[newi] = '\\'; //set the next character of newstr to \\
        }
    }
    if (newstr[newi] != '\0') newstr[newi] = '\0'; //make sure newstr is null-terminated
}

int strSlashesLen(char *str) //ascertains length of str, if all instances of \\ were replaced with \\\\
{
    int len = 0;
    for (int i = 0; str[i]; ++i)
    {
        ++len; //increment length for each character
        if (str[i] == '\\') ++len; //if current character is a \\ then increment space for another \\
    }
    return len;
}

the replacing of \ with \\ appears to be necessary; otherwise the files refuse to open (winxp)

Dave Sinkula · #5 14-May-2005, 17:25

Quote:

Originally Posted by ubergeek

the replacing of \ with \\ appears to be necessary; otherwise the files refuse to open (winxp)

Or just use forward slashes.

ubergeek · #6 14-May-2005, 19:08

interestingly enough, I'm pretty sure I tried that too. Maybe I just didn't implement the code properly, or it could have been a compiler peculiarity (Dev-C++ 4.9.9.2). Either way, I might try it again--It would save a lot of memory because / is the same number of characters as \, so I wouldn't have to allocate a whole new string.

Dave Sinkula · #7 15-May-2005, 15:57

What's with the embedded nulls in the string?

CPP / C++ / C Code:

sfn.lpstrFilter = "Text Files\0*.txt\0GeekEdit Files\0*.gke\0All Files\0*.*\0";

ubergeek · #8 15-May-2005, 16:53

that's Microsoft for you. Those are the items that will appear in the Save as Type drop-down in the SaveAs... dialog box. It will appear as "Text Files" "Geekedit Files" "All Files" (in a drop-down). GetSaveFileName() parses that string like this:
For first item, scan until first \0. From there until the next \0 is the filter to associate with that item. (as in: text files are *.txt). Etc. To avoid buffer overruns, the function needs to know where the string ends, hence the \0 at the end. Normally when you type something like

CPP / C++ / C Code:

char stuff[] = "Hello";

the compiler automatically adds a trailing \0 to your string literal. GetSaveFileName expects two \0s at the end of the filter string, so that it knows to stop parsing. Therefore, I put in one of my own and allowed the compiler to insert the second.

MSDN on GetSaveFileName:
http://msdn.microsoft.com/library/de...vefilename.asp
and the OPENFILENAME structure, where lpstrFilter is discussed:
http://msdn.microsoft.com/library/de...enfilename.asp

Quote:

Originally Posted by MSDN

lpstrFilter
Pointer to a buffer containing pairs of null-terminated filter strings. The last string in the buffer must be terminated by two NULL characters.

The first string in each pair is a display string that describes the filter (for example, "Text Files"), and the second string specifies the filter pattern (for example, "*.TXT"). To specify multiple filter patterns for a single display string, use a semicolon to separate the patterns (for example, "*.TXT;*.DOC;*.BAK"). A pattern string can be a combination of valid file name characters and the asterisk (*) wildcard character. Do not include spaces in the pattern string.

Dave Sinkula · #9 15-May-2005, 18:02

Quote:

Originally Posted by ubergeek

GetSaveFileName expects two \0s at the end of the filter string, so that it knows to stop parsing.

I thought I remembered it being something like that, but I couldn't quite remember. Thanks for the refresher.

Thread Tools	Search this Thread
Show Printable Version Email this Page	Search this Thread: Advanced Search
Rate This Thread
Rate This Thread:

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
How to check a string value read from an edit box	E2STE	MS Visual C++ / MFC Forum	1	06-May-2005 01:19
Help! Some basal questions about MFC	xutingnjupt	MS Visual C++ / MFC Forum	1	05-Dec-2004 03:38
Detecting a mouse click outside of control	arran.s	MS Visual C++ / MFC Forum	1	27-Oct-2004 06:01
RichEdit Demo	Max Payne	MS Visual C++ / MFC Forum	3	06-Oct-2004 07:19