Reputation: 4338
I was trying to write a simple client server program on windows. My Server side code looks like below .
#include <iostream>
#include <windows.h>
#include <winsock2.h>
#include <stdlib.h>
#include <stdio.h>
using namespace std;
int main()
{
WSADATA WSAData;
SOCKET serverSock, clientSock;
SOCKADDR_IN serverAddr, clientAddr;
WSAStartup(MAKEWORD(2,0),&WSAData);
serverSock=socket(AF_INET,SOCK_STREAM,0);
serverAddr.sin_addr.s_addr=INADDR_ANY;
serverAddr.sin_family=AF_INET;
serverAddr.sin_port=htons(5555);
bind(serverSock,(SOCKADDR *)&serverAddr, sizeof(serverAddr));
listen(serverSock,0);
std::cout<<"Listening for incoming connections \n";
char buffer[1024];
//memset(buffer,0,sizeof(buffer));
int clientAddrsize= sizeof(clientAddr);
if(clientSock = ( accept(serverSock,(SOCKADDR *)&clientAddr, &clientAddrsize ) != INVALID_SOCKET) )
{
std::cout<<"Client connected \n";
recv(clientSock,buffer,int(strlen(buffer)),0);
std::cout<<"Client Says: " << buffer <<"\n" ;
Sleep(10000);
//memset(buffer,0,sizeof(buffer));
closesocket(clientSock);
closesocket(serverSock);
WSACleanup();
}
return 0;
}
Client side code looks like:
#include <iostream>
#include <winsock2.h>
#include <stdlib.h>
#include <stdio.h>
using namespace std;
int main()
{
WSADATA WSAData;
SOCKET clientSock;
SOCKADDR_IN addr;
WSAStartup(MAKEWORD(2,0),&WSAData);
clientSock=socket(AF_INET,SOCK_STREAM,0);
addr.sin_addr.s_addr=inet_addr("127.0.0.1");
addr.sin_family=AF_INET;
addr.sin_port=htons(5555);
connect(clientSock,(SOCKADDR *)&addr,sizeof(addr));
std::cout <<"Connected to the server \n";
//memset(buffer,0,sizeof(buffer));
char buffer[1024]={'h','e','l','l','o','.','\0'};
send(clientSock,buffer,int(strlen(buffer)),0);
std::cout<<"Message sent \n" ;
Sleep(10000);
closesocket(clientSock);
WSACleanup();
std::cout<<"Socket closed \n";
return 0;
}
I see below output on server side:
Listening for incoming connections
Client connected
Client Says: R☺
What i was expecting was Client Says: Hello.
. Not sure why i am getting the garbage value.
Thanks
Upvotes: 1
Views: 399
Reputation: 118445
char buffer[1024];
This declares a local char
array in automatic scope. The array is not initialized anything, and contains random garbage.
recv(clientSock,buffer,int(strlen(buffer)),0);
This passes buffer
to strlen
. However, buffer
contains garbage, this is undefined behavior, with the eventual results could be, randomly, anything between an immediate crash, at this point, or some bizarre return value from strlen
.
Additionally, the return value of recv
is completely ignored and not checked at all.
All socket system calls can fail, and every socket system call, send
, recv
, and all others can fail. Additionally, even if it succeeds, recv
gives you no guarantees whatsoever that everything that was send
in one shot, will be recv
ed, in one shot.
If, for example, send
sent 10 bytes, the first call to recv
might return 1 or 5, indicating that only the first one or five bytes were immediately recv
ed. You are not guaranteed that recv
will receive those 10 bytes all at once. Otherwise, recv
will (obviously) need to be called again to receive the rest of the message.
You will need to fix all of these issues in order to have a reliable client/server application:
Check the return value of all system calls. Everything that I said about recv
also applies to send
as well. An attempt to send
10 bytes might come back with, say, 7, indicating that only the first 7 bytes were sent, and you must then implement the appropriate logic to send the remaining bytes, too.
Fix undefined behavior, of passing an uninitialized value to strlen
.
Upvotes: 1