Socket Class - Accept Forum

[UnfitElf]

Hi people,

This is the first class that i have ever tried to do and i only started to understand the concept a few days ago, so if im doing anything all that is "bad" or "good" please post your comments.

As far as i can tell from the limited testing that i have done all the client functions are working really good.. im able to connect to a server and send/recieve strings of text.

I then moved onto writing the server application however i have some very strange results when i get to the "accept" stage of the program.. i cant really explane what is happening becuase im not 100% sure, if i was i would fix it. Its like the program gets shutdown when it gets to the "accept" stage. printf()s that are placed after the accept stage dont show.

Anyway i realy hope someone can help me out.. here is the code:

CSocket.h
#include <winsock.h>

class CSocket
{
private:
// The WSADATA structure contains information about the Windows Sockets implementation
static int numOfSockets;
static WSADATA winsockData;
static bool Initilized;

//The SOCKET variable holds the descriptor referencing the new socket
SOCKET ConnSocket;
//The LPHOSTENT variable is a pointer to the host information returned by gethostbyname
LPHOSTENT hostEntry;
//The SOCKADDR_IN structure is used by Windows Sockets to specify a local or remote endpoint address to which to connect a socket
SOCKADDR_IN ServerAddr;
public:
CSocket (int lowByte, int highByte); //Version number E.G. "1.2" is lowbyte 1 highbyte 2
~CSocket ();
bool Create(int type, int protocol); //SOCK_STREAM or SOCK_DGRAM, IPPROTO_TCP or IPPROTO_UDP
bool GetHost(char *host); //E.G. "

http://www.hal-pc.org"

bool Bind(int portNum);
bool Listen(int backlog); //Maximum length of the queue of pending connections
SOCKET Accept(void);
bool Connect(int portNum);
bool Send(char *message);
bool Recieve(char *message, int size); //size refers to the size of 'message'
bool Close(void);
int num(void);
};

CSocket.cpp
#include <stdio.h>
#include "CSocket.h"

int CSocket::numOfSockets = 0;
bool CSocket::Initilized = false;
WSADATA CSocket::winsockData;
int CSocket::num(void)
{
return numOfSockets;
}

CSocket::CSocket(int lowByte, int highByte)
{
numOfSockets++;
if(CSocket::Initilized == false)
{
printf("WSAStartup Called\n\n");
if(WSAStartup(MAKEWORD(lowByte, highByte), &CSocket::winsockData) == 0)
{
//Check for correct version
if(LOBYTE(CSocket::winsockData.wVersion) == lowByte || HIBYTE(CSocket::winsockData.wVersion) == highByte)
{
printf("Winsock version OK!\n\n");
CSocket::Initilized = true;

// We must set the ServerAddr structure to 0 because if not the some undesired effects can occure
memset(&ServerAddr, 0, sizeof(ServerAddr));
// Setting settings that are always the same
ServerAddr.sin_family = AF_INET;
}
}
}
}

CSocket::~CSocket(void)
{
numOfSockets--;
if(numOfSockets == 0)
{
printf("WSACleanup");
WSACleanup();
}
}

bool CSocket::Create(int type, int protocol)
{
ConnSocket = socket(AF_INET, type, IPPROTO_TCP);
if (ConnSocket != INVALID_SOCKET)
{
return true;
}
return false;
}

bool CSocket::GetHost(char *host)
{
unsigned long addr = inet_addr(host);
if(addr == INADDR_NONE)
{
hostEntry = gethostbyname(host);
}
else
{
hostEntry = gethostbyaddr((char*)&addr, sizeof(addr), AF_INET);
}

if(hostEntry == NULL)
return false;

ServerAddr.sin_addr = *((LPIN_ADDR)*hostEntry->h_addr_list);
return true;
}

bool CSocket::Bind(int portNum)
{
ServerAddr.sin_port = htons(portNum);

ServerAddr.sin_addr.s_addr = INADDR_ANY; // Since this socket is listening for connections,
// any local address will do
if(bind(ConnSocket, (SOCKADDR*)&ServerAddr, sizeof(ServerAddr)) != SOCKET_ERROR)
{
return true;
}
return false;
}

bool CSocket::Listen(int backlog)
{
if(listen(ConnSocket, backlog) != 0)
{
return true;
}
return false;
}

SOCKET CSocket::Accept(void)
{
printf( "Waiting for a client to connect...\n" );
while (1)
{
SOCKET AcceptSocket = SOCKET_ERROR;
while(AcceptSocket == SOCKET_ERROR)
{
printf("Loop Start\n\n");
//This is where im having the problem, "Loop Start" is printed, however "Loop End" is not

AcceptSocket = accept(ConnSocket, NULL, NULL);
printf("LoopEnd\n\n");
}
printf( "Client Connected.\n");
ConnSocket = AcceptSocket;
break;
}
//
return accept(ConnSocket, NULL, NULL);
}

bool CSocket::Connect(int portNum)
{
ServerAddr.sin_port = htons(portNum);

if(connect(ConnSocket, (SOCKADDR*)&ServerAddr, sizeof(ServerAddr)) != SOCKET_ERROR)
{
return true;
}
return false;
}

bool CSocket::Recieve(char *message, int size)
{
int value = recv(ConnSocket, message, size, 0);
if((value != SOCKET_ERROR) && (value != 0))
{
return true;
}
return false;
}

bool CSocket::Send(char *message)
{
int value = send(ConnSocket, message, strlen(message), 0);
if(value != SOCKET_ERROR)
{
return true;
}
return false;
}

bool CSocket::Close(void)
{
closesocket(ConnSocket);
}

Thanks heaps for anyhelp

 

[ArkM]

It's a low level wrapper for socket. If no incoming connections you may wait on your Accept() forever.
1. Use <winsock2.h> instead of old <winsock.h>.
2. Don't mix SOCKET_ERROR and INVALIS_SOCKET (they both are 0xFFFFFFFFu in Winsock but they are logically different things - for example, accept() returns INVALID_SOCKET but not SOCKET_ERROR).
3. Present you test case code and scenario...

 

[UnfitElf]

Thanks, for the reply, i will look into what u suggest

 

[cpjust]

I'm not very familiar with network programming either, but here's what I found:
In Accept(void), you can get rid of the outer while(1) since it can never loop more than once because of the break anyways.
Then at the end of the function you have:

return accept(ConnSocket, NULL, NULL);

which should be:

return AcceptSocket;

I think the return value may be your problem since you wait for one connection with the first accept(), then you wait again for another connection on the return.

Other than that, you may also consider changing your printf()'s to cout's, use C++ casts like static_cast<char*> instead of C-style (char*) casts. Also check if NULL pointers and other invalid data are being passed to the functions (or at least use assert's).

In Create(int type, int protocol) you could also change from int's to enums or objects that wrap only valid types and protocols so there is less room for error from the users of this class.