What's wrong with this code, take two...

  • Article

Well, my last “What’s wrong with this code” post was such a rollicking good success, I figured I’d do it one more time (I’m SUCH a glutton for punishment).

This time, I not only bench checked the code, but I verified that it actually worked (which is more than I can say for the last one) J.

Basically this is a routine that receives a “message” from the network. The “message” is to be time stamped and sequenced on arrival (this allows the message to be posted to worker threads while preserving ordering semantics). A “message” consists of a short containing the length of the message and a buffer containing the actual contents of the message. 

Just for reference, there are four intentional bugs in this code. Tomorrow, I’ll post what the bugs are.       

Assume that there is only one thread calling ReceiveSocketMessage on a particular socket at any given time – in other words, the logic involving the sequence numbers and/or timestamps isn’t the source of the bugs J.

/*++
* Network Message – Holds a message received from the network.
*
*--*/
typedef struct _NetworkMessage
{
long MessageSequence;
FILETIME MessageTime;
unsigned short MessageSize;
char Message[ANYSIZE_ARRAY];
} NETMSG, *PNETMSG;
long GlobalMessageSequence = 0; // Global variable containing the current sequence count.
/*++
* ReceiveSocketMessage
*
* Receives a message from a socket.
*
* A “message” consists of a short message length and a buffer containing the actual message.
*
* Inputs:
* socket – Opened socket on which to receive the message.
* ppnetmsg – Output pointer that will hold a newly allocated PNETMSG structure with the MessageSequence, MessageTime, MessageSize and Message fields filled in.
*
* Returns:
* Winsock result code if error, 0 if success.
*
*--*/
int ReceiveSocketMessage(SOCKET socket, PNETMSG *ppnetmsg)
{
int cbReceived = SOCKET_ERROR;
short cbMessage;
PNETMSG pnetmsg;
assert(ppnetmsg != NULL);
//
// Receive the byte count from the socket.
//
cbReceived = recv(socket, (char *)&cbMessage, sizeof(short), 0);
if (cbReceived == SOCKET_ERROR)
{
return WSAGetLastError();
}
//
// Allocate a buffer to hold the network message.
//
pnetmsg = (PNETMSG)new BYTE[cbReceived + FIELD_OFFSET(NETMSG, Message)];
if (pnetmsg == NULL)
{
//
// Couldn't allocate the buffer for the socket.
//
return WSAENOBUFS;
}
//
// Fill in the static header values.
//
pnetmsg->MessageSequence = InterlockedIncrement(&GlobalMessageSequence);
GetSystemTimeAsFileTime(&pnetmsg->MessageTime);
pnetmsg->MessageSize = cbMessage;
//
// Receive the actual buffer from the server.
//
cbReceived = recv(socket, (char *)&pnetmsg->Message, pnetmsg->MessageSize, 0);
if (cbReceived == SOCKET_ERROR)
{
return WSAGetLastError();
}
//
// If we didn't get the amount requested, return an error.
//
if (cbReceived != cbMessage)
{
return WSAEMSGSIZE;
}
//
// Return success.
//
*ppnetmsg = pnetmsg;
return 0;
}