Winsock 2, bind problem & WSAGetLastError

[TheBreadCat]

I have a function that takes my a Socket object and binds it to a port.
But the bind function returns an SOCKET_ERROR.
When I try to call WSAGetLastError it just returns 0.

void ServerSocket::bind()
{
	if (index < 0 || index > 99) throw NetCodeException("Index out of bounds( 0-99 )", "bind()");
	address = new INetAddress("127.0.0.1",2700 + index);
	int r = ::bind(sock, (const struct sockaddr*) address->address, sizeof (struct sockaddr_in));
	if (r == SOCKET_ERROR)
	{
		delete address;
		throw SocketException(WSAGetLastError(), "bind()");
	}
	bound = true;
}

...

try
{
	ServerSocket * ss = new ServerSocket(0);
	ss->bind(0);
	ss->listen(5);
	Socket* s = ss->accept();
	Packet pack;
	pack.data = (byte*)"Hello wob.";
	pack.size = 10;
	ss->send(s,&pack);
	Packet* p = ss->recv(s);
	printf("Received %s\n",p);
	ss->close(s);
	ss->stop();
	delete p;
} catch (const SocketException &x)
{
	print(x.what());
	return;
}

...

INetAddress::INetAddress(const char* ip, short port)
{
	address = new sockaddr_in;
	memset(address, 0, sizeof *address);
	address->sin_family = AF_INET;
	address->sin_port = port;
	address->sin_addr.S_un.S_addr = inet_addr(ip);
	if (address->sin_addr.S_un.S_addr == 0 || address->sin_addr.S_un.S_addr == (unsigned int) -1) //Not an IP
	{
		struct hostent *he;
		struct in_addr **addr_list;
		if ((he = gethostbyname( ip )) == 0)
		{
			throw SocketException(WSAGetLastError(),"gethostbyname()" );
			delete address;
		}
		addr_list = (struct in_addr**) he->h_addr_list;
		for(int i = 0; addr_list[i] != 0; i++)
		{
			address->sin_addr = *addr_list[i];
			break;
		}
	}
}

[TheBreadCat]

Bump.

[kbw]

For the life or me, I don't know why you allocate address on the heap. It's small enough to be sensibly handled by value. Further more, you don't show its declaration, so we can't be sure what it is.

You shouldn't be checking the address against 0 and -1. You really mean INADDR_NONE and INADDR_ANY, so use that.

If you're going to abstract an internet address, I'd expect it to be an interface that has two concrete types (IPv4 and IPv6). So you wouldn't be accessing public data, it would provide methods to get the data and the size of that data. Public data? Just say no.

I recommend you remove that INetAddress stuff and just use conventional sockaddr_in and intitialise it in the standard way.

[TheBreadCat]

InetAddress:

class INetAddress : public MapObject
{
public:
	INetAddress(const char*,short);
	INetAddress(byte*,int,short);
	INetAddress(short);
	sockaddr_in * address;
};

I can see your point on the address being a pointer, but writing constants literary instead of by name yield the same. The abstraction is both to remove boilerplate code and to make it interface with the scripting engine.

And please respond if you actually have a solution to this problem.

[coder777]

You shouldn't call anything between the reception of the error and WSAGetLastError()
-> put WSAGetLastError() before delete address;


How is sock created?

[kbw]

As coder777 asked, How is sock created?


You declare:

void ServerSocket::bind()

But you use:

ServerSocket * ss = new ServerSocket(0);
ss->bind(0);

Clearly, that shouldn't even compile. And what is 0?


If you have to call bind/listen/accept/read/write on ServerSocket, why bother with it. It doesn't save you anything, you may as well use socket functions directly.


Why does INetAddress derive from MapObject?


Why does INetAddress have a pointer to a sockaddr_in, why not just hold the value?


If your convoluted code doesn't work, strip out that fluff, get it working and put it all back in again. Clearly you've made a mistake, but it's impossible to see what it is from what you've posted.

[TheBreadCat]

ServerSocket::ServerSocket(int i)
{
	ServerSocket();
	if (i < 0 || i > 99) throw NetCodeException("Index out of bounds( 0-99 )", "ServerSocket()");
	index = i;
}

ServerSocket::ServerSocket()
{
	sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); // TCP
	if (sock == INVALID_SOCKET)
	{
		closesocket(sock);
		throw SocketException(WSAGetLastError(), "socket()");
	}
}

void ServerSocket::bind(short i)
{
	index = i;
	bind();
}

void ServerSocket::bind()
{
	if (index < 0 || index > 99) throw NetCodeException("Index out of bounds( 0-99 )", "bind()");
	address = new INetAddress(2700 + index);
	int r = ::bind(sock, (const struct sockaddr*)  &(address->address), sizeof (struct sockaddr_in));
	if (r == SOCKET_ERROR)
	{
		throw SocketException(WSAGetLastError(), "bind()");
	}
	bound = true;
}

INetAddress derives from MapObject becuase MapObject is a script object, it makes the scripting engine interface with INetAddress.

The pointer in INetAddress is my coding fault and has been fixed:

void ServerSocket::bind()
{
	if (index < 0 || index > 99) throw NetCodeException("Index out of bounds( 0-99 )", "bind()");
	address = new INetAddress(2700 + index);
	int r = ::bind(sock, (const struct sockaddr*)  &(address->address), sizeof (struct sockaddr_in));
	if (r == SOCKET_ERROR)
	{
		throw SocketException(WSAGetLastError(), "bind()");
	}
	bound = true;
}

And the reason why the ServerSocket exists is both for less boilerplate code and for the scripting engine to be able to interface with it.

[coder777]

So, is your problem solved now?

ServerSocket::ServerSocket()
{
	sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); // TCP
	if (sock == INVALID_SOCKET)
	{
		closesocket(sock); // remove this, closing a non existing socket makes no sense + WSAGetLastError() doesn't work
		throw SocketException(WSAGetLastError(), "socket()");
	}
}

[TheBreadCat]

It doesn't change anything, and the problem didn't even occur in ServerSocket(), as stated in the OP it is the bind() function the unwanted behavior occurs.

[coder777]

The problem isn't that bind() fails. The problem is that WSAGetLastError() returns 0.
It looks like if something resets the error.

From you code snippets it's not possible to reproduce the problem

[TheBreadCat]

try
	{
		SOCKET sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); // TCP
		if (sock == INVALID_SOCKET)
		{
			throw SocketException(WSAGetLastError(), "socket()");
		}
		struct sockaddr_in address;
		address.sin_family = AF_INET;
		address.sin_port = htons(2700);
		address.sin_addr.S_un.S_addr = ADDR_ANY;
		if (::bind(sock, (const struct sockaddr*) &(address), sizeof(struct sockaddr_in)) == SOCKET_ERROR)
		{
			throw SocketException(WSAGetLastError(), "bind()");
		}
		struct sockaddr_in readdr;
		readdr.sin_family = AF_INET;
		int ll = sizeof(struct sockaddr_in);
		SOCKET remote = ::accept(sock,(sockaddr*)&readdr,&ll);
		if(remote == INVALID_SOCKET)
		{
			throw SocketException(WSAGetLastError(),"accept()");
		}
		printf("ll: %d",ll);
		if (::send(remote, (char*) "Hello wob.", 10, 0) == SOCKET_ERROR) throw SocketException(WSAGetLastError(), "send()");
		byte * data[128];
		if (::recv(sock, (char*) data, 128, 0) == SOCKET_ERROR)
		{
			int lerr = WSAGetLastError();
			throw SocketException(lerr, "recv()");
		}
		printf("Received %s\n",data);
		closesocket(remote);
		closesocket(sock);
	} catch (const SocketException &x)
	{
		print(x.what());
		return;
	}

Cut all the function code out, and removed all the OO. Now I get the error code 0 from accept().

[TheBreadCat]

bump

[kbw]

Now, isn't that clearer?

Are you saying accept() returns a socket value of -1 and then WSAGetLastError() then returns zero?

This:

SOCKET sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

should be:

SOCKET sock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);

This is unncessary:

readdr.sin_family = AF_INET;

Also, this:

byte * data[128];

should be:

byte data[128];

I take it you're calling WSAStartUp() somewhere before all this.

[TheBreadCat]

try
	{
		SOCKET sock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
		if (sock == INVALID_SOCKET)
		{
			throw SocketException(WSAGetLastError(), "socket()");
		}
		struct sockaddr_in address;
		address.sin_family = AF_INET;
		address.sin_port = htons(2700);
		address.sin_addr.S_un.S_addr = ADDR_ANY;
		if (::bind(sock, (const struct sockaddr*) &(address), sizeof(struct sockaddr_in)) == SOCKET_ERROR)
		{
			throw SocketException(WSAGetLastError(), "bind()");
		}
		struct sockaddr_in readdr;
		int ll = sizeof(struct sockaddr_in);
		SOCKET remote = ::accept(sock,(sockaddr*)&readdr,&ll);
		if(remote == INVALID_SOCKET)
		{
			throw SocketException(WSAGetLastError(),"accept()");
		}
		printf("ll: %d",ll);
		if (::send(remote, (char*) "Hello wob.", 10, 0) == SOCKET_ERROR) throw SocketException(WSAGetLastError(), "send()");
		byte data[128];
		if (::recv(sock, (char*) data, 128, 0) == SOCKET_ERROR)
		{
			int lerr = WSAGetLastError();
			throw SocketException(lerr, "recv()");
		}
		printf("Received %s\n",data);
		closesocket(remote);
		closesocket(sock);
	} catch (const SocketException &x)
	{
		print(x.what());
		return;
	}

None of your suggestions work, and WSAStartUp() is being called.
WSAStartup(MAKEWORD(2,2), &wsaData);

[kbw]

I didn't say they'd fix your problem, they were just other things that were wrong.

I asked for clarification on what the problem is, but you haven't answered that. It's not really clear what the problem is.

[TheBreadCat]

As said, my problem is that accept() fails and WSAGetLastError() returns 0.

[kbw]

You're not calling listen(). You need to call it after bind() and before accept().

Have you noticed that bind() isn't failing anymore?

[TheBreadCat]

Oh bloody hell why did I forget that -_-'
And it works, thanks for pointing it out for me.