Codechange: remove public access to the next pointer in Packet

src/network/core/packet.cpp

@@ -62,6 +62,32 @@ Packet::~Packet()
 	free(this->buffer);
 }
 
+/**
+ * Add the given Packet to the end of the queue of packets.
+ * @param queue  The pointer to the begin of the queue.
+ * @param packet The packet to append to the queue.
+ */
+/* static */ void Packet::AddToQueue(Packet **queue, Packet *packet)
+{
+	while (*queue != nullptr) queue = &(*queue)->next;
+	*queue = packet;
+}
+
+/**
+ * Pop the packet from the begin of the queue and set the
+ * begin of the queue to the second element in the queue.
+ * @param queue  The pointer to the begin of the queue.
+ * @return The Packet that used to be a the begin of the queue.
+ */
+/* static */ Packet *Packet::PopFromQueue(Packet **queue)
+{
+	Packet *p = *queue;
+	*queue = p->next;
+	p->next = nullptr;
+	return p;
+}
+
+
 /**
  * Writes the packet size from the raw packet from packet->size
  */

src/network/core/packet.h

@@ -62,6 +62,9 @@ public:
 	Packet(PacketType type);
 	~Packet();
 
+	static void AddToQueue(Packet **queue, Packet *packet);
+	static Packet *PopFromQueue(Packet **queue);
+
 	/* Sending/writing of packets */
 	void PrepareToSend();
 

src/network/core/tcp.cpp

@@ -42,9 +42,7 @@ NetworkRecvStatus NetworkTCPSocketHandler::CloseConnection(bool error)
 
 	/* Free all pending and partially received packets */
 	while (this->packet_queue != nullptr) {
-		Packet *p = this->packet_queue->next;
+		delete Packet::PopFromQueue(&this->packet_queue);
-		delete this->packet_queue;
-		this->packet_queue = p;
 	}
 	delete this->packet_recv;
 	this->packet_recv = nullptr;
@@ -60,21 +58,10 @@ NetworkRecvStatus NetworkTCPSocketHandler::CloseConnection(bool error)
  */
 void NetworkTCPSocketHandler::SendPacket(Packet *packet)
 {
-	Packet *p;
 	assert(packet != nullptr);
 
 	packet->PrepareToSend();
-
+	Packet::AddToQueue(&this->packet_queue, packet);
-	/* Locate last packet buffered for the client */
-	p = this->packet_queue;
-	if (p == nullptr) {
-		/* No packets yet */
-		this->packet_queue = packet;
-	} else {
-		/* Skip to the last packet */
-		while (p->next != nullptr) p = p->next;
-		p->next = packet;
-	}
 }
 
 /**
@@ -96,8 +83,7 @@ SendPacketsState NetworkTCPSocketHandler::SendPackets(bool closing_down)
 	if (!this->writable) return SPS_NONE_SENT;
 	if (!this->IsConnected()) return SPS_CLOSED;
 
-	p = this->packet_queue;
+	while ((p = this->packet_queue) != nullptr) {
-	while (p != nullptr) {
 		res = p->TransferOut<int>(send, this->sock, 0);
 		if (res == -1) {
 			int err = GET_LAST_ERROR();
@@ -120,9 +106,7 @@ SendPacketsState NetworkTCPSocketHandler::SendPackets(bool closing_down)
 		/* Is this packet sent? */
 		if (p->RemainingBytesToTransfer() == 0) {
 			/* Go to the next packet */
-			this->packet_queue = p->next;
+			delete Packet::PopFromQueue(&this->packet_queue);
-			delete p;
-			p = this->packet_queue;
 		} else {
 			return SPS_PARTLY_SENT;
 		}

src/network/network_server.cpp

@@ -79,9 +79,7 @@ struct PacketWriter : SaveFilter {
 		/* This must all wait until the Destroy function is called. */
 
 		while (this->packets != nullptr) {
-			Packet *p = this->packets->next;
+			delete Packet::PopFromQueue(&this->packets);
-			delete this->packets;
-			this->packets = p;
 		}
 
 		delete this->current;
@@ -132,11 +130,7 @@ struct PacketWriter : SaveFilter {
 	{
 		std::lock_guard<std::mutex> lock(this->mutex);
 
-		Packet *p = this->packets;
+		return Packet::PopFromQueue(&this->packets);
-		this->packets = p->next;
-		p->next = nullptr;
-
-		return p;
 	}
 
 	/** Append the current packet to the queue. */
@@ -144,12 +138,7 @@ struct PacketWriter : SaveFilter {
 	{
 		if (this->current == nullptr) return;
 
-		Packet **p = &this->packets;
+		Packet::AddToQueue(&this->packets, this->current);
-		while (*p != nullptr) {
-			p = &(*p)->next;
-		}
-		*p = this->current;
-
 		this->current = nullptr;
 	}
 
@@ -158,7 +147,8 @@ struct PacketWriter : SaveFilter {
 	{
 		if (this->current == nullptr) return;
 
-		this->current->next = this->packets;
+		/* Reversed from AppendQueue so the queue gets added to the current one. */
+		Packet::AddToQueue(&this->current, this->packets);
 		this->packets = this->current;
 		this->current = nullptr;
 	}