Sockets programming in C++

[Xprotocol]

I'm trying to make a simple Windows-based chat program using socket programming in C++. The client connects to the server fine but when either trys to send information, I get an illegal operation on that end and the program shuts down. I was thinking it might have something to do with the order in which data is sent or recieved. If someone sends data to a connected computer, is it stored in some sort of memory cache and then accessed when the program makes the appropriate function call to get input? Or does this command have to be constantly looking for data from another system? Maybe someone can figure something out from this code of the mainloop() function.

int MainLoop()
{

char oData[255] = {0}; //for our outgoing data
char *iData = NULL; //for our incoming data

while(true)
{ //loop forever, you can add a break

cout << endl << "Command: "; //Ask for input
cin >> oData; //get the input

if(!myWS.SendData(oData))
{//if our data was not sent then show error
cout << endl << "There was an error sending data!" << endl;
}

iData = myWS.GetData(); //Get any incoming data
//Note, this can be improved greatly!
//so make sure you improve it.

cout << "Data: " << iData; //print the data that was received
}

return true;
}


The error occurs when either system enters something after "command:" is printed to the screen and input to oData is requested. If you need to see any other portion of the code let me know. Thanks for any help in advance.

[megadave]

I'm trying to make a simple Windows-based chat program

justlinux.com.....just the place to post my windows programming question.

just linux...linux only. An exclusive statement, no?

not maybelinux.com or ambiguous.com.

Just kidding. I think.
-----------------------------------------------------------------------------------

Just so this is not a useless post. I know that the NIC has a very small cache that will hold a tiny bit of info, but maybe the OS buffers more, I'm not sure.

[Xprotocol]

Being that C++ is for the most part, pretty much the same through-out different OS's. No one really bugs me about Windows programming here and are usually able to help me out.

[busa_blade]

Is that same code being run on both sides of the connection?

[Xprotocol]

Yes, once a connection is established, each side (whether in client or server mode) is directed to the mainloop() function.

[egh01]

Do you think you can post your "myWS"-class code? Atleast the code for the connection establishing and for the operations Get-/SendData.

[bwkaz]

Originally posted by Xprotocol
char oData[255] = {0}; Hmm... I'm not sure this is right. You might want to, instead of that, do something like:

char oData[255];

oData[0] = '\0'; i.e. do the initialization later. My understanding of the initialization stuff, the way you've got it set up, is that the compiler creates an array of 255 char's on the stack, then assigns oData to the start of an array of 1 int -- the {0}. This might cause issues later on when one of the functions you call writes into oData expecting it to be longer than it actually is (i.e. the >> function).

You're also wasting 255 char's worth of stack space that way, again, if my understanding of the initializer is right.

[Xprotocol]

Here's the class code. Note: this code wasn't written by me so I may not be able to explain some of the comments :p


/************************************************** ******************\
Description: This file holds the functions to our cWinSock class
defined in cWinSock.h header file.

I researched all of this code and wrote it by hand. I copied a few
bits of code, but had to modify it so much, it might as well have
been original. It was real hard learning all of this crap, so enjoy!
\************************************************* *******************/

#include "Global.h"

/************************************************** ******************\
Listen()

This function listens on a specified port.
\************************************************* *******************/
int cWinSock::Listen(int Port)
{

closesocket(sckListen); //Close the socket in case its open
sckListen = socket(AF_INET,SOCK_STREAM,0); //create a valid socket from our
//blank socket and set the data
//type. SOCK_STREAM = TCP
//SOCK_DGRAM = UDP

memset(&sin,0,sizeof(sin)); //clears our SOCKADDR_IN struct

sin.sin_family = AF_INET; //I dont know :(
sin.sin_addr.s_addr = INADDR_ANY; //This specifies any address, only
//good for listening.
sin.sin_port = htons( Port ); //Specifies the port

//We bind our local address to our socket for listening.
if ( bind( sckListen, (SOCKADDR FAR*)&sin, sizeof(sin) ) == SOCKET_ERROR )
{
return FALSE;
}

//infinate loop until a connection is requested.
while(true)
{
if(listen(sckListen,0) != SOCKET_ERROR)
{ //look for connection
AcceptConnection(); //If we got one, accept it!
break;
}
}

return true;
}

/************************************************** ******************\
AcceptConnection()

This function will accept a connection request
\************************************************* *******************/
int cWinSock::AcceptConnection() {

int Length = sizeof(sin);

closesocket(sckConnection); //close socket if its open

//Accept the connection coming in on our listen socket, and set our
//blank socket to a valid one.
sckConnection = accept(sckListen,(SOCKADDR FAR*)&sin,&Length);

return true;
}

/************************************************** ******************\
CheckVersion()

This function checks for version 2 of winsock and if it is the right
version, it initializes it.
\************************************************* *******************/
int cWinSock::CheckVersion() {

WORD Version;
int Error;

Version = MAKEWORD(2,0);

Error = WSAStartup(Version,&wsaData); //Initialize WSA (i dunno any more)

if(Error == SOCKET_ERROR) { //if it didnt initialize error out
return false;
}

return true;
}

/************************************************** ******************\
Connect()

This function connects to a specified IP / Port
\************************************************* *******************/
int cWinSock::Connect(char *IPAddress,int Port) {

closesocket(sckConnection); //close the socket
sckConnection = socket(AF_INET,SOCK_STREAM,0); //create a valid socket from our
//blank socket and set the data
//type. SOCK_STREAM = TCP
//SOCK_DGRAM = UDP

memset(&sin,0,sizeof(sin)); //clears our SOCKADDR_IN struct

sin.sin_family = AF_INET; //I dont know :(
sin.sin_addr.s_addr = inet_addr(IPAddress); //sets the IP address
sin.sin_port = htons( Port ); //sets the port

//trys to make a connection using the info we set above
if(connect(sckConnection,(SOCKADDR FAR*)&sin,sizeof(SOCKADDR_IN)) == SOCKET_ERROR) {
return false;
}

return true;
}

/************************************************** ******************\
SendData()

this function will send the specified data over the open connection
\************************************************* *******************/
int cWinSock::SendData(char *strData) {

int Length = strlen(strData);

if(sckConnection == INVALID_SOCKET) { //make sure we have an open connection
return false;
}

send(sckConnection,strData,Length,SO_SNDTIMEO); //send the data using our socket

return true;
}
/************************************************** ******************\
GetData()

This function retreives the incoming data from the open connection
\************************************************* *******************/
char *cWinSock::GetData() {

char IncomingData[255] = {0};

if(sckConnection == INVALID_SOCKET) { //make suer we have an open connection
return false;
}

//gets the data usin gour socket and stores it in our variable
if(recv(sckConnection,IncomingData,sizeof(Incoming Data),SO_RCVTIMEO) == SOCKET_ERROR) {
return false;
}

//return the data
return IncomingData;

}

/************************************************** ******************\
~cWinSock()

This deconstructor runs the code to shut down everything
\************************************************* *******************/
cWinSock::~cWinSock() {
closesocket(sckConnected); //close the socket
WSACleanup(); //shuts down WSA ??

}

[egh01]

I don't know if I got you right, does the error occure before, while or after you've enter the input data?
If it happens after you've entered a '\n' (new line) character, check what:
send(sckConnection,strData,Length,SO_SNDTIMEO); returns

[bwkaz]

Try running this under a debugger. I bet the error happens in the cin >> oData; line, not the myWS.SendData() line...

[Stuka]

Can you post what you have above your mainLoop()? I've tinkered a bit with Winsock stuff, and it MAY be in your setup that the errors occur.

[Xprotocol]

The error occurs after a succesful client-server connection has been made and data is entered into oData and the enter key is hit.

"Can you post what you have above your mainLoop()? "

#include "Global.h" //Includes all of our includes

#pragma comment( lib, "ws2_32.lib" ) //link to the winsock2 library

cWinSock myWS; //Create our winsock made ez class

int client();
int server();
int MainLoop();

void main()
{

if(!myWS.CheckVersion())
{
cout << endl << "Winsock initialization error! Wrong version" << endl;
system("PAUSE");
}

int choice;
for(;;)
{
cout << "A simple server/client chat system using Winsocks.\n";
cout << "\nEnter number for selection.";
cout << "\n1: Client mode";
cout << "\n2: Server mode";
cout << "\n3: Exit";
cout << "\nChoice: ";
cin >> choice;
switch (choice)
{
case 1: client();
break;
case 2: server();
break;
case 3: exit(1);
break;
default: cout << "Invaild selection, please re-select.\n";
system("PAUSE");
}//End switch
}//End for
}



int client()
{

char IP[16] = {0}; //our IP address information
int Port; //our port information

system("cls"); //clear the console screen

//Ask for an IP address
cout << "Enter the IP address (eg. xxx.xxx.xxx.xxx)" << endl << "IP: ";
cin >> IP;

//Ask for a port
cout << "Enter the Port (eg. 6666)" << endl << "Port: ";
cin >> Port;

system("cls");
cout << "Trying to connect...";

if(!myWS.Connect(IP,Port))
{//Try to connect, if it dont work then
cout << "Failed!"; //error out.
return false;
}

cout << "Connected!" << endl;

MainLoop(); //If it did conenct then we need
//to get/send/receive data
return true;
}


int server()
{
int Port; //our port number

cout << "Enter the Port (eg. 6666)" << endl << "Port: "; //Ask for a port
cin >> Port; //Get the port

system("cls"); //ez clear console screen
cout << "Waiting for someone to connect...";

myWS.Listen(Port); //This listens on the port until a
//connection is requested

MainLoop(); //a connection was made ok, so now
//we need to get/send/receive data

return true;

}

[egh01]

Have you checked what is returned from call to 'send()'?

[bwkaz]

Originally posted by bwkaz
Try running this under a debugger. I bet the error happens in the cin >> oData; line, not the myWS.SendData() line... Have you tried this? I hate to keep quoting myself, but it seems like I'm getting ignored, and I think this could be the answer...

[Stuka]

I MAY have found the problem, and it's not in your code! Your code does this: cout << endl << "Command: "; //Ask for input
cin >> oData; //get the input

if(!myWS.SendData(oData))
{//if our data was not sent then show error
cout << endl << "There was an error sending data!" << endl;
}

iData = myWS.GetData(); //Get any incoming data which, AFAICT, is good. However, in the GetData() function, there's a SERIOUS problem: char IncomingData[255] = {0};
...
return IncomingData; //HUGE ERROR!! GetData() is returning the address of a LOCAL VARIABLE. VC++ 6 issues a warning on this...but I assure you, it's far more of an error than a warning!! You're PROBABLY getting an illegal operation because you've got a bad pointer.

[Xprotocol]

Originally posted by bwkaz
Have you tried this? I hate to keep quoting myself, but it seems like I'm getting ignored, and I think this could be the answer...

the debugger points to the line right below main_loop:

main_loop:
00405380 mov eax,dword ptr [ecx]

[Xprotocol]

I tried a quick fix, making char IncomingData[255] = {0}; a global variable in cWinSock.cpp, it got rid of VC++'s warning but the error still happens in the same place, whether the server or the client tries to send data.

[bwkaz]

D'oh! I forgot -- Visual Studio's debugger is a POS. Where in the C code is that assembly line?

[Xprotocol]

Not sure, it says no source file and just beeps at me when I say goto source :mad:

[bwkaz]

Might it help to go up a couple of stack frames? The actual problem might be happening in a library routine or something, that it doesn't have source for.

[mindcooler]

Post links to the *complete source code* with clear instructions on how to reproduce the error and I will have a look. Not promising any answers, but I will take a look.

[Xprotocol]

Originally posted by mindcooler
Post links to the *complete source code* with clear instructions on how to reproduce the error and I will have a look. Not promising any answers, but I will take a look.

The link for source code download can be found here: http://www.planetsourcecode.com/vb/scripts/ShowCode.asp?txtCodeId=5405&lngWId=3. This is the base code that I am using for my program. I made some changes in mine but nothing that messes with the network communication portion of it. Plus, his pure code has the same problem in the same place (and yes I have tried a different machine). To get to the error, do the following. To make things easy, you can do it on one system (doesn't make a difference if its one or two computers, same problem). Compile and run, select the "wait for connection" option, and choose a port number. Now run a seccond instance of the program, select connect, and now type in the loopback address (127.0.0.1) and then the port number you selected. After you connect, both programs will acknowledge the connection and give a "command:" prompt. Now type something in, into either the client or server side and hit enter. You should now get an illegal opertion error.

[mindcooler]

That's just the library? I said I wanted the complete source code. Including what you did.

[Xprotocol]

Originally posted by bwkaz
Might it help to go up a couple of stack frames? The actual problem might be happening in a library routine or something, that it doesn't have source for.

ostream::writepad(const char * 0x0042b44c `string', const char * 0x00000000) line 143 + 9 bytes
ostream::operator<<(const char * 0x00000000) line 62

I'm guessing these stack frames are from iostream.h. There is of course the KERNAL stack frame as well but everything else points to the original source code.

[mindcooler]

Ok, I've done some checking of the code now. First of I must say that the code is very poorly written. The author clearly has very little experience in C++. I don't know sockets very well at all (a complete beginner, I must admit), but I quickly noticed that listen() will return immediately if you set the program to wait for a connection even if you haven't launched another instance of the program. So the socket code is seriously flawed. Regarding GetData() returning address of a local variable, I solved that by making it take a char** as an argument. Ideally, you will want GetData() to determine how much data there is and allocate memory for it accordingly. Alternatively, you could use std::string references (or pointers). But fixing GetData() is of no use right now since the basic framework for listening for connection requests doesn't work. I suggest getting a good book or tutorial and create your own framework, because the library code is buggy indeed. And it seems you could use some practice with the debugger for not spotting these errors.

[bwkaz]

Originally posted by Xprotocol
ostream::writepad(const char * 0x0042b44c `string', const char * 0x00000000) line 143 + 9 bytes
ostream::operator<<(const char * 0x00000000) line 62 Oh yay. Someone's trying to write a NULL pointer out a stream somwhere. You're not ever doing that, so it must be the library you're using.

Try what mindcooler said. I believe that the code in Beej's network programming guide works on Win32 as long as you include the ws2_32.h header, or something named very similar to that. Google for "Beej's network", and you'll probably find it.

It was written to use the Unix syscalls, but the ws2_32.h header (or whatever it is) implements those as Winsock calls.

[mindcooler]

Yes, it is a null pointer that is causing the crash. The offending line is:
cout << "Data: " << iData;
in MainLoop(). iData is NULL because the call to GetData() fails. This is not related to GetData(), in its originial form, returning the address of a local variable, but that's an error that needs to be handled too, of course. To be honest, I don't see how this program is supposed to work as you want without the use of threads. An exception class socket_error should be created and the socket class should throw such exceptions when errors occur.