patterncppMinor
TCP socket retry mechanism
Viewed 0 times
sockettcpretrymechanism
Problem
I am writing a C++ client application which will send some data to server and wait for its response. Now the protocol is to wait for a specific timeout and then retry for specific times. If all goes wrong, the client will report a communication failure.
I have implemented the whole matter in a non blocking socket operation. I have some doubts on whether my send/receive methods are correct or not.
Below is my code for TCP communication which is written in VC++ 2005 on Windows platform.
Socket parameters building method
Connection building method
```
bool CTCPCommunication::Connect()
{
ReportStatus(App_Stat_Connect);
CT2CA serverIP(m_csServerIP);
char* pchserverIP = serverIP;
WRITELOG("Connecting to server %s:%d", pchserverIP, m_iServe
I have implemented the whole matter in a non blocking socket operation. I have some doubts on whether my send/receive methods are correct or not.
Below is my code for TCP communication which is written in VC++ 2005 on Windows platform.
Socket parameters building method
bool CTCPCommunication::OpenConnection(bool bRetryConnect)
{
//create the socket handle and config its paramaters
m_hSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (m_hSocket == INVALID_SOCKET)
{
WRITELOG("Call to API 'socket' failed, error: %d", WSAGetLastError());
return false;
}
//setting socket address
CT2CA serverIP(m_csServerIP);
char* pchServerIP = serverIP;
m_stAddress.sin_family = AF_INET;
m_stAddress.sin_addr.S_un.S_addr = inet_addr(pchServerIP);
m_stAddress.sin_port = htons(m_iServerPort);
//setting socket timeout
m_stTimeout.tv_sec = SOCK_TIMEOUT_SECONDS;
m_stTimeout.tv_usec = 0;
//set socket to non blocking mode
unsigned long iMode = 1;
int iResult = ioctlsocket(m_hSocket, FIONBIO, &iMode);
if (iResult != NO_ERROR)
{
WRITELOG("Call to API 'ioctlsocket' failed, error: %d", WSAGetLastError());
return false;
}
bool bSuccess = false;
//Called for the first time when starting server connection
if (bRetryConnect == false)
{
bSuccess = InitialConnect();
}
//For all the other time when client detects a failure in communication and makes a retry
else
{
bSuccess = Connect();
}
return bSuccess;
}Connection building method
```
bool CTCPCommunication::Connect()
{
ReportStatus(App_Stat_Connect);
CT2CA serverIP(m_csServerIP);
char* pchserverIP = serverIP;
WRITELOG("Connecting to server %s:%d", pchserverIP, m_iServe
Solution
First, you should not compare values to boolean literals in an
Second, you are modifying a local variable right here, which will go out of scope as soon as you leave the method:
Right after this, you break out of your loop and
Third, you have this snippet:
This can be written without
The parenthesis around the first statement are not necessary, but might help readability.
if statement. This is better off written as bSuccess instead of bSuccess == true and !bSuccess instead of bSuccess == false.Second, you are modifying a local variable right here, which will go out of scope as soon as you leave the method:
else
{
WRITELOG("Call to socket API 'select' failed inside recv method, error: %d", WSAGetLastError());
bSuccess = false;
break;
}Right after this, you break out of your loop and
return bSuccess;. This could be written to demonstrate you are immediately returning a fail signal like this:else
{
WRITELOG("Call to socket API 'select' failed inside recv method, error: %d", WSAGetLastError());
return false;
}Third, you have this snippet:
//decide success or failure
if((iRet > 0) && (FD_ISSET(m_hSocket, &fdWrite)))
{
return true;
}
return false;This can be written without
ifs like this:return (iRet > 0) && FD_ISSET(m_hSocket, &fdWrite);The parenthesis around the first statement are not necessary, but might help readability.
Code Snippets
else
{
WRITELOG("Call to socket API 'select' failed inside recv method, error: %d", WSAGetLastError());
bSuccess = false;
break;
}else
{
WRITELOG("Call to socket API 'select' failed inside recv method, error: %d", WSAGetLastError());
return false;
}//decide success or failure
if((iRet > 0) && (FD_ISSET(m_hSocket, &fdWrite)))
{
return true;
}
return false;return (iRet > 0) && FD_ISSET(m_hSocket, &fdWrite);Context
StackExchange Code Review Q#88396, answer score: 5
Revisions (0)
No revisions yet.