Change write_all such that it will always return "size" or -1 (error)

* Check for write_all() == -1 (error) instead of "< 0" and "!= size" to
   see whether sending has failed.
 * Don't just mark sockets as "invalid" on error (by setting them to
   NULL), close them as well.
 * Give decent error messages on write failure (most likely a disconnect)

Addresses #435

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/trunk@7288 4a71c877-e1ca-e34f-864e-861f7616d084
master
Giel van Schijndel 2009-05-03 15:16:34 +00:00 committed by Git SVN Gateway
parent 61e0a18a6a
commit ae878a7880
1 changed files with 54 additions and 53 deletions

View File

@ -218,11 +218,6 @@ static ssize_t write_all(Socket* sock, const void* buf, size_t size)
while (written < size) while (written < size)
{ {
ssize_t ret = write(sock->fd, &((char*)buf)[written], size - written); ssize_t ret = write(sock->fd, &((char*)buf)[written], size - written);
if (ret == 0)
{
return written;
}
if (ret == -1) if (ret == -1)
{ {
switch (errno) switch (errno)
@ -232,8 +227,6 @@ static ssize_t write_all(Socket* sock, const void* buf, size_t size)
continue; continue;
default: default:
if (written)
return written;
return -1; return -1;
} }
} }
@ -1171,7 +1164,7 @@ static void NETsendGAMESTRUCT(Socket* sock, const GAMESTRUCT* game)
// Send over the GAMESTRUCT // Send over the GAMESTRUCT
result = write_all(sock, buf, sizeof(buf)); result = write_all(sock, buf, sizeof(buf));
if (result != sizeof(buf)) if (result == -1)
{ {
// If packet could not be sent, we should inform user of the error. // If packet could not be sent, we should inform user of the error.
debug(LOG_ERROR, "Failed to send GAMESTRUCT. Reason: %s", strerror(errno)); debug(LOG_ERROR, "Failed to send GAMESTRUCT. Reason: %s", strerror(errno));
@ -1204,10 +1197,11 @@ static bool NETrecvGAMESTRUCT(GAMESTRUCT* game)
|| !tcp_socket->ready || !tcp_socket->ready
|| (result = read_all(tcp_socket, buf, sizeof(buf))) != sizeof(buf)) || (result = read_all(tcp_socket, buf, sizeof(buf))) != sizeof(buf))
{ {
if (result < 0) if (result == -1)
{ {
debug(LOG_WARNING, "SDLNet_TCP_Recv error: %s tcp_socket %p is now invalid", strerror(errno), tcp_socket); debug(LOG_WARNING, "Server socket ecountered error: %s", strerror(errno));
tcp_socket = NULL; //SDLnet docs say to 'disconnect' here. Dunno how. :S SocketClose(tcp_socket);
tcp_socket = NULL;
} }
return false; return false;
} }
@ -1533,15 +1527,12 @@ BOOL NETsend(NETMSG *msg, UDWORD player)
nStats.packetsSent += 1; nStats.packetsSent += 1;
return true; return true;
} }
else else if (result == -1)
{ {
if (result < size) // Write error, most likely client disconnect.
{ debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
// TCP_Send error, inform user of problem. This really should never happen normally. SocketClose(connected_bsocket[player]->socket);
// Possible client disconnect, and in either case, socket is most likely invalid now. connected_bsocket[player]->socket = NULL;
debug(LOG_WARNING, "SDLNet_TCP_Send returned: %d error: %s line %d", result, strerror(errno), __LINE__);
connected_bsocket[player]->socket = NULL ; // It is invalid, Unknown how to handle.
}
} }
} }
else else
@ -1552,15 +1543,12 @@ BOOL NETsend(NETMSG *msg, UDWORD player)
{ {
return true; return true;
} }
else else if (result == -1)
{ {
if (result < size) // Write error, most likely client disconnect.
{ debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
// TCP_Send error, inform user of problem. This really should never happen normally. SocketClose(tcp_socket);
// Possible client disconnect, and in either case, socket is most likely invalid now. tcp_socket = NULL;
debug(LOG_WARNING, "SDLNet_TCP_Send returned: %d error: %s line %d", result, strerror(errno), __LINE__);
tcp_socket = NULL; // unknow how to handle when invalid.
}
} }
} }
@ -1604,15 +1592,14 @@ BOOL NETbcast(NETMSG *msg)
// FIXME: We are NOT checking checkSockets/SDLNet_SocketReady // FIXME: We are NOT checking checkSockets/SDLNet_SocketReady
// SDLNet_TCP_Send *can* block! // SDLNet_TCP_Send *can* block!
result = write_all(connected_bsocket[i]->socket, msg, size); result = write_all(connected_bsocket[i]->socket, msg, size);
if (result < size) if (result == -1)
{ {
// TCP_Send error, inform user of problem. This really should never happen normally. // Write error, most likely client disconnect.
// Possible client disconnect, and in either case, socket is most likely invalid now. debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
debug(LOG_WARNING, "(server) SDLNet_TCP_Send returned %d < %d, socket %p invalid: %s",
result, size, connected_bsocket[i]->socket, strerror(errno));
players[i].heartbeat = false; //mark them dead players[i].heartbeat = false; //mark them dead
debug(LOG_WARNING, "Player (dpid %u) connection was broken.", i); debug(LOG_WARNING, "Player (dpid %u) connection was broken.", i);
connected_bsocket[i]->socket = NULL; // Unsure how to handle invalid sockets. SocketClose(connected_bsocket[i]->socket);
connected_bsocket[i]->socket = NULL;
} }
} }
} }
@ -1626,13 +1613,13 @@ BOOL NETbcast(NETMSG *msg)
// FIXME: We are NOT checking checkSockets/SDLNet_SocketReady // FIXME: We are NOT checking checkSockets/SDLNet_SocketReady
// SDLNet_TCP_Send *can* block! // SDLNet_TCP_Send *can* block!
result = write_all(tcp_socket, msg, size); result = write_all(tcp_socket, msg, size);
if (result < size) if (result == -1)
{ {
// I believe host has dropped...here // Write error, most likely client disconnect.
debug(LOG_WARNING, "(client) SDLNet_TCP_Send returned %d < %d, tcp_socket %p is now invalid: %s", debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
result, size, tcp_socket, strerror(errno));
debug(LOG_WARNING, "Host connection was broken?"); debug(LOG_WARNING, "Host connection was broken?");
tcp_socket = NULL; // unsure how to handle invalid sockets. SocketClose(tcp_socket);
tcp_socket = NULL;
players[HOST_DPID].heartbeat = false; //mark them dead players[HOST_DPID].heartbeat = false; //mark them dead
//Game is pretty much over --should just end everything when HOST dies. //Game is pretty much over --should just end everything when HOST dies.
return false; return false;
@ -1964,7 +1951,13 @@ receive_message:
&& connected_bsocket[j] != NULL && connected_bsocket[j] != NULL
&& connected_bsocket[j]->socket != NULL) && connected_bsocket[j]->socket != NULL)
{ {
write_all(connected_bsocket[j]->socket, pMsg, size); if (write_all(connected_bsocket[j]->socket, pMsg, size) == -1)
{
// Write error, most likely client disconnect.
debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
SocketClose(connected_bsocket[pMsg->destination]->socket);
connected_bsocket[pMsg->destination]->socket = NULL;
}
} }
} }
} }
@ -1981,10 +1974,12 @@ receive_message:
pMsg->size = ntohs(pMsg->size); pMsg->size = ntohs(pMsg->size);
result = write_all(connected_bsocket[pMsg->destination]->socket, pMsg, size); result = write_all(connected_bsocket[pMsg->destination]->socket, pMsg, size);
if (result < size) if (result == -1)
{ {
debug(LOG_WARNING,"SDLNet_TCP_Send(line %d) failed, because of %s", __LINE__, strerror(errno)); // Write error, most likely client disconnect.
connected_bsocket[pMsg->destination]->socket = NULL; // socket becomes invalid on error debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
SocketClose(connected_bsocket[pMsg->destination]->socket);
connected_bsocket[pMsg->destination]->socket = NULL;
} }
} }
else else
@ -2324,12 +2319,10 @@ static void NETallowJoining(void)
int result = 0; int result = 0;
debug(LOG_NET, "cmd: list. Sending game list"); debug(LOG_NET, "cmd: list. Sending game list");
result = write_all(tmp_socket[i], &numgames, sizeof(numgames)); result = write_all(tmp_socket[i], &numgames, sizeof(numgames));
if( result < sizeof(numgames) ) if (result == -1)
{ {
// TCP_Send error, inform user of problem. This really should never happen normally. // Write error, most likely client disconnect.
// Possible client disconnect, and in either case, socket is most likely invalid now. debug(LOG_ERROR, "Failed to send message: %s", strerror(errno));
// How to handle error?
debug(LOG_WARNING, "SDLNet_TCP_Send returned: %d error: %s line %d", result, strerror(errno),__LINE__);
debug(LOG_WARNING, "Couldn't get list from server. Make sure required ports are open. (TCP 9998-9999)"); debug(LOG_WARNING, "Couldn't get list from server. Make sure required ports are open. (TCP 9998-9999)");
} }
else else
@ -2371,8 +2364,16 @@ static void NETallowJoining(void)
if (size <= 0) if (size <= 0)
{ {
// socket probably disconnected. // disconnect or programmer error
debug(LOG_NET, "tmp socket %p probably disconnected", tmp_socket[i]); if (size == 0)
{
debug(LOG_NET, "Client socket disconnected.");
}
else
{
debug(LOG_NET, "Client socket ecountered error: %s", strerror(errno));
}
delSocket(tmp_socket_set, tmp_socket[i]); delSocket(tmp_socket_set, tmp_socket[i]);
SocketClose(tmp_socket[i]); SocketClose(tmp_socket[i]);
tmp_socket[i] = NULL; tmp_socket[i] = NULL;
@ -2648,11 +2649,11 @@ BOOL NETfindGame(void)
} }
else else
{ {
if (result < 0) if (result == -1)
{ {
debug(LOG_NET, "SDLNet_TCP_Recv returned %d, error: %s - tcp_socket %p is now invalid.", debug(LOG_NET, "Server socket ecountered error: %s", strerror(errno));
result, strerror(errno), tcp_socket); SocketClose(tcp_socket);
tcp_socket = NULL; // unsure how to handle invalid sockets? tcp_socket = NULL;
} }
// when we fail to receive a game count, bail out // when we fail to receive a game count, bail out
setLobbyError(ERROR_CONNECTION); setLobbyError(ERROR_CONNECTION);