deleting TcpSocket pointer from QList on server when disconnecting

I've got a server that every time a new connection comes in, the pointer to that socket is added to a struct which is added to a QList. The socket is actually part of a struct, but I don't' think that makes a difference. When the socket deletes, I want to remove the struct from my list so that my list of structs doesn't end up with a bunch of empty holes in it but still delete the socket properly. I don't want any dangling pointers. It seems to work, but if I don't have it right, it might cause memory leaks and I don't want that. If it was a normal pointer I would know what to do, but with sockets, timing matters (I think?)

Code:

---

//.h
struct struct_ClientSocket
{
	QTcpSocket *pClientSocket;
	QString str_ClientAddress;
	int nPort;
};
 
private:
        QList<struct_ClientSocket> m_lst_pClientSockets;
 
private slots:
	void SetIncomingSocket();
	void OnSocketDisconnected();
	void ReadInputData();
 
//.cpp
void CKeyboardLogAdmin::SetIncomingSocket()
{
	QStringList str_lst_Name;
	QTcpSocket *tempSocket = pTcpServer->nextPendingConnection();
	struct_ClientSocket temp_struct_ClientSocket;
	temp_struct_ClientSocket.pClientSocket = tempSocket;
	temp_struct_ClientSocket.str_ClientAddress = tempSocket->peerAddress().toString();
	temp_struct_ClientSocket.nPort = tempSocket->peerPort();
	connect(tempSocket, SIGNAL(disconnected()), tempSocket, SLOT(deleteLater()));
	connect(tempSocket, SIGNAL(disconnected()), this, SLOT(OnSocketDisconnected()));
	connect(tempSocket, SIGNAL(readyRead()), this, SLOT(ReadInputData()));
	m_lst_pClientSockets.append(temp_struct_ClientSocket);
}
 
void CKeyboardLogAdmin::OnSocketDisconnected()
{
	struct_ClientSocket tempSocket;
	for (int nSock = 0; nSock < m_lst_pClientSockets.size(); nSock++)
	{
		if (m_lst_pClientSockets[nSock].pClientSocket->state() == QAbstractSocket::UnconnectedState || m_lst_pClientSockets[nSock].pClientSocket->state() == QAbstractSocket::ClosingState)
		{
			m_lst_pClientSockets.removeAt(nSock);
			break;
		}
	}
}
 
void CKeyboardLogAdmin::ReadInputData()
{
	m_str_ReadData.clear();
	QByteArray ba_ByteData;
	int nSocket;
	if (m_lst_pClientSockets.size() > 0)
	{
		for (nSocket = 0; nSocket < m_lst_pClientSockets.size(); nSocket++)
		{
			if (m_lst_pClientSockets[nSocket].pClientSocket->bytesAvailable() > 0)
			{
				break;
			}
		}
		if (nSocket < m_lst_pClientSockets.size())
		{
			while (m_lst_pClientSockets[nSocket].pClientSocket->bytesAvailable() > 0)
			{
				ba_ByteData = m_lst_pClientSockets[nSocket].pClientSocket->readAll();
			}
			const char* ch_Data = ba_ByteData.data();
			m_str_ReadData = QString(ch_Data);
			QStringList str_lst_ReadData = m_str_ReadData.split("\t");
			if (!str_lst_ReadData.isEmpty())
			{
				if (str_lst_ReadData.first().compare(tr("LoginUser")) == 0)
				{
					ProcessLoginUserData(str_lst_ReadData);
				}
				if (str_lst_ReadData.first().compare(tr("LoginGuests")) == 0)
				{
					ProcessLoginGuests(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("Logout")) == 0)
				{
					ProcessLogoutData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("ScreenLock")) == 0)
				{
					ProcessScreenLockData((str_lst_ReadData));
				}
				else if (str_lst_ReadData.first().compare(tr("ScreenUnlock")) == 0)
				{
					ProcessScreenUnlockData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("AccountLogout")) == 0)
				{
					ProcessComputerUserAccountLogoutData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("ValidatePin")) == 0)
				{
					ProcessValidatePinData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("SessionLogoff")) == 0)
				{
					ProcessSessionLogoff(str_lst_ReadData);
				}
			}
		}
	}
}

---

I would swap lines 22 and 23; connections are executed in the order they are made, so it would be best to ensure your disconnection cleanup code is executed prior to calling deleteLater() on the socket instance.

You might also find QPointer or QSharedPointer useful to avoid dangling references. You can also connect to the QObject::destroyed() signal instead of disconnect().

Is something like this what you were talking about?

Code:

---

//.h
struct struct_ClientSocket
{
	QTcpSocket *pClientSocket;
	QString str_ClientAddress;
	int nPort;
};
 
private:
	QList<struct_ClientSocket> m_lst_pClientSockets;
 
private slots:
	void SetIncomingSocket();
	void OnSocketDestroyed();
	void ReadInputData();
 
//.cpp
void CKeyboardLogAdmin::SetIncomingSocket()
{
	QStringList str_lst_Name;
	QTcpSocket *tempSocket = pTcpServer->nextPendingConnection();
	struct_ClientSocket temp_struct_ClientSocket;
	temp_struct_ClientSocket.pClientSocket = tempSocket;
	temp_struct_ClientSocket.str_ClientAddress = tempSocket->peerAddress().toString();
	temp_struct_ClientSocket.nPort = tempSocket->peerPort();
	connect(tempSocket, SIGNAL(destroyed(QObject*)), this, SLOT(OnSocketDestroyed(QObject*)));
	connect(tempSocket, SIGNAL(disconnected()), tempSocket, SLOT(deleteLater()));
	connect(tempSocket, SIGNAL(readyRead()), this, SLOT(ReadInputData()));
	m_lst_pClientSockets.append(temp_struct_ClientSocket);
}
 
 
 
 
void CKeyboardLogAdmin::OnSocketDestroyed(QObject *obj)
{
	for (int nSock = 0; nSock < m_lst_pClientSockets.size(); nSock++)
	{
		if (m_lst_pClientSockets[nSock].pClientSocket == obj)
		{
			m_lst_pClientSockets.removeAt(nSock);
			break;
		}
	}
}
 
 
 
 
void CKeyboardLogAdmin::ReadInputData()
{
	m_str_ReadData.clear();
	QByteArray ba_ByteData;
	int nSocket;
	if (m_lst_pClientSockets.size() > 0)
	{
		for (nSocket = 0; nSocket < m_lst_pClientSockets.size(); nSocket++)
		{
			if (m_lst_pClientSockets[nSocket].pClientSocket->bytesAvailable() > 0)
			{
				break;
			}
		}
		if (nSocket < m_lst_pClientSockets.size())
		{
			while (m_lst_pClientSockets[nSocket].pClientSocket->bytesAvailable() > 0)
			{
				ba_ByteData = m_lst_pClientSockets[nSocket].pClientSocket->readAll();
			}
			const char* ch_Data = ba_ByteData.data();
			m_str_ReadData = QString(ch_Data);
			QStringList str_lst_ReadData = m_str_ReadData.split("\t");
			if (!str_lst_ReadData.isEmpty())
			{
				if (str_lst_ReadData.first().compare(tr("LoginUser")) == 0)
				{
					ProcessLoginUserData(str_lst_ReadData);
				}
				if (str_lst_ReadData.first().compare(tr("LoginGuests")) == 0)
				{
					ProcessLoginGuests(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("Logout")) == 0)
				{
					ProcessLogoutData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("ScreenLock")) == 0)
				{
					ProcessScreenLockData((str_lst_ReadData));
				}
				else if (str_lst_ReadData.first().compare(tr("ScreenUnlock")) == 0)
				{
					ProcessScreenUnlockData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("AccountLogout")) == 0)
				{
					ProcessComputerUserAccountLogoutData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("ValidatePin")) == 0)
				{
					ProcessValidatePinData(str_lst_ReadData);
				}
				else if (str_lst_ReadData.first().compare(tr("SessionLogoff")) == 0)
				{
					ProcessSessionLogoff(str_lst_ReadData);
				}
			}
		}
	}
}

---

Something like that should work. I don't think there was anything wrong with your original solution (after lines swapped); I was just suggesting alternative ways to skin the same cat.