Why Diff in outp W32 Cons Vs Dlg

I am confused, too many of you thats not a surprising statement... :-)

I've been trying to learn serial port programing and threading, and so far its been ok till now, but recently I just got baffled, I have never had this happen....

I got help coding a Win32 console app that reads my serial port and I programed a device, basic stamp, where I could get the exact output I want to repeat the message "Hello" . I refined the code below until I got exactly the message reads I wanted from the serial port and then output that to a file. I won't include the function definitions unless necessary to conserve time, but certainly don't mind if it helps, I am just including the execution of the functions and output: Win32 Console App

Code:

int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
{
	int nRetCode = 0;
	//open port and configure it
	hPort = ConfigureSerialPort("COM1");

	//declare data variables for read string function
	char message[4];
	int sizeBuffer =4;
	int count = 0;


	//check to see if configure serial port failed
	if(hPort==NULL)
	{
//		cout<<"COM port config failed"<<endl;
		return -1;
	}
	else
	{
		ofstream fout;
		fout.open("C:\\rs232test.txt", ios::app);
		do
		{
			ofstream fout;
			fout.open("C:\\rs232test.txt");
			//call readfile function
			ReadString(message, &sizeBuffer);
			cout<<message<<endl;
			fout<<message<<endl;
			count++;
			Sleep(46);
			fout.close();
		}while (count <=100);
	} 

	//call the close port function
	ClosePort();

	return nRetCode;
}

OUTPUT From Console app:

Code:

ÌÌÌÌ
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO
HELLO

Same Code in a MFC DLG application, upon clicking the OK button, runs for a few seconds, then app crashes and closes automatically, output file very different

Code:

 void CSerialTestDlg::OnOK() 
{
	// TODO: Add extra validation here
		hPort = ConfigureSerialPort("COM1");
	Sleep(1000);

	//declare data variables for read string function
	char message[4];
	int sizeBuffer =4;
	int count = 0;


	//check to see if configure serial port failed
	if(hPort==NULL)
	{
//		cout<<"COM port config failed"<<endl;
		return;
	}
	else
	{
		ofstream fout;
		fout.open("C:\\ThreadTest.txt", ios::app);
		do
		{
			//call readfile function
			ReadString(message, &sizeBuffer);
			fout<<message<<endl;
			Sleep(46);
			count++;
			Sleep(46);
			if (count == 100)
			{
				AfxMessageBox("Loop Finished");
				fout.close();
				break;
			}
		}while (count <=100);
	} 
	
	AfxMessageBox("Calling ClosePort");
	//call the close port function
	ClosePort();
	
//	CDialog::OnOK();
}

OUTPUT from DLG app:

Code:

HELLOHELLO

Now, I know there are some very different message handling concepts between a MFC app and basic Win32 Console app, but can someone tell me why I would be getting such dramatic results?

hmmm... ok added some code to both console app and dlg app and got some interesting results

decided I wanted to see where in my loop the readstring function was so added the count variable to my output file

New code in both apps:

Code:

fout<<message<<count<<endl;

Console app results:

Code:

ÌÌÌÌ0
HELLO1
HELLO2
HELLO3
HELLO4
HELLO5
HELLO6
HELLO7
HELLO8
HELLO9

DLG app

Code:

HELLOHELLOHELLOHELLOHELLOHELLOHELLOHELLO

I added an extended Sleep to my DLG app and was able to get more output from the port before the app crashed. Howerver, it appears that the ReadString function never returns, it enters the loop on the first pass and just reads the port continously until it over runs the memory set aside for the declared variable then crashed the app.....

It looks like you have a buffer overflow bug in the ReadString function. "HELLO" will not fit in a four character buffer. Including the nul terminator, you will need at least six characters.

As to why it works in a console but not GUI, that's the nature of undefined behaviour, sometimes it works, othertimes it doesn't.

I tried to increase the size of message [] in the calling function but still got the same result.

Code:

char message [6];

Reading your reply it looks like I need to define the size in the function definition, being as I am using code someone helped me with, not quite sure I understand that, below is the readstring definition:

Code:

//the read string function
BOOL ReadString(void *outstring, int *length)
{
    BYTE data;
    BYTE dataout[4096]={0};
    int index = 0;
    while(ReadByte(data)== TRUE)
    {
        dataout[index++] = data;
    }
    memcpy(outstring, dataout, index);
    *length = index;
    return TRUE;
}

outstring is the returing string, which message is getting this value in the function call, dataout is the array collecting the incoming bytes from the called function ReadByte, one byte at a time, So do I need to increase the size of dataout to accomodate a larger memory buffer?

Thanks for the help-

I think the problem is that message is not being nul terminated. Here is how I would write the ReadString function:

Code:

BOOL ReadString(void *outstring, int *length)
{
    BYTE  data;
    BYTE* dataout = outstring; /* Put the data straight into the user supplied buffer. */
    int   index   = 0;

    /* Make sure we at least have room for a nul terminator. */
    if (*length <= 0) 
    {
        *length = 0;
        return FALSE;
    }

    /* Do not write past the end of the user buffer and leave space for nul terminator. */
    while(index < *length - 1 && ReadByte(data) == TRUE)
    {
        dataout[index++] = data;
    }

    /* Nul terminate the string. */
    dataout[index++] = '\0';

    *length = index; /* Returned length includes the nul terminator. */
    return TRUE;
}

anontymouse's ReadString function is the correct way to write that code. In the future, I'll have to review my code more carefully before I post it.

Thanks to both of you for your replies, Bob, your code helped me get where I am so far and Anonytmous your assistance is helping me understand my errors writing back to the dialog.

I posted the code you sent and I get 2 errors, both related to the same thing, the line it refers to having the issue with is:

Code:

BYTE* dataout = outstring; /* Put the data straight into the user supplied buffer. */

the error message is:

Code:

error C2440: 'initializing' : cannot convert from 'void *' to 'unsigned char *'
        Conversion from 'void*' to pointer to non-'void' requires an explicit cast

I apologize for my lacking uderstanding of declared variables that are being called, I do not uderstand what the diffrence is.... Thanks again for all the help-

OK, getting better, I was able to use both examples and come up with the code below, which compiles and reads the port and does not crash the app, so definately progress!

Code:

BOOL ReadString(void *outstring, int *length)
{
    BYTE  data;
    BYTE dataout[4096]= {0}; /* Put the data straight into the user supplied buffer. */
    int   index   = 0;

    /* Make sure we at least have room for a nul terminator. */
    if (*length <= 0) 
    {
        *length = 0;
        return FALSE;
    }

    /* Do not write past the end of the user buffer and leave space for nul terminator. */
    while(index < *length - 1 && ReadByte(data) == TRUE)
    {
        dataout[index++] = data;
    }

    /* Nul terminate the string. */
    dataout[index++] = '\0';

	memcpy(outstring, dataout, index);


    *length = index; /* Returned length includes the nul terminator. */
    return TRUE;
}

I still am not seeing the exact results I am expecting with resutls of Hello1 ect... but I am getting this instead:

Code:

HELLOHELL0
OHELLOHEL1
LOHELLOHE2
LLOHELLOH3
ELLOHELLO4
HELLOHELL5
OHELLOHEL6
LO7
8
9
10
11
12
13
14
15
16
17

it finishes to the the end of the loop of 50 and then stops appropriately. Which is a great improvement. Not I just need to get the consistenancy in the response, like I do in the Win32 Console app, which always works exactly the same every time.....

>> I posted the code you sent and I get 2 errors <<

Yep, I was coding in C, while clearly I should have been coding in C++. The error can be fixed with a simple cast, however your code should be fine.

Code:

   BYTE* dataout = (BYTE*) outstring;

I'm not sure why you are getting the output shown. It appears to be returning 9 characters in every call to ReadString. Are you using a ten character buffer? You have an extra Sleep call in your GUI version. I suspect this means that you are getting more input each time with the UI version rather than only getting one "HELLO" each time with the console version. So a couple of things to try:
1. Use a large buffer, say 1000 characters, so that you know that the ReadString function is returning everything it can rather than being limited by the buffer size.
2. Use identical timing in the UI version as the console version.

hey hey hey, we got it!!! I took your advice and played with the sleep times and i got the buffer equal to what the byte string was + null terminator and here are our results:

Code:

HELLO0
HELLO1
HELLO2
HELLO3
HELLO4
HELLO5
HELLO6
HELLO7
HELLO8
HELLO9
HELLO10
HELLO11
HELLO12
HELLO13
HELLO14
HELLO15

new code with Sleeps

Code:

	// TODO: Add extra validation here
		hPort = ConfigureSerialPort("COM1");
	Sleep(1000);

	//declare data variables for read string function
	char message [6];
	int sizeBuffer =6;
	int count = 0;


	//check to see if configure serial port failed
	if(hPort==NULL)
	{
//		cout<<"COM port config failed"<<endl;
		return;
	}
	else
	{
//		AfxMessageBox("Running ReadString");
		ofstream fout;
		fout.open("C:\\ThreadTest.txt", ios::app);
		do
		{
			//call readfile function
			ReadString(message, &sizeBuffer);
			fout<<message<<count<<endl;
			Sleep(600);
			count++;
			if (count == 50)
			{
				AfxMessageBox("Loop Finished");
				fout.close();
				break;
			}
		}while (count <=50);
	} 
	
	AfxMessageBox("Calling ClosePort");
	//call the close port function
	ClosePort();

Next step returning, putting my thread function back in and returning the value to edit box within dialog. Thank you very much for the assistance.

Awesome, got the thread working, and returning smoothly to the dlg window edit box!

I am so stoked, that if the weather was not so cold in Texas right now, I would drop the boat in the lake and grab some serious air on the wakeboard... hence the Alias: WaterNut

Thanks again for the help to all -

I'm sure I will require more, but that totally helps-

WATERNUT-