From 3d7372d529b147edbd4a1efe5ffe88af592bacaa Mon Sep 17 00:00:00 2001
From: Richard Cochran <richardcochran@gmail.com>
Date: Sat, 7 Jul 2012 20:10:28 +0200
Subject: [PATCH] Fix memory leak, reference counting, and list handling in
 message code.

The message code is horribly broken in three ways.

1. Clearing the message also sets the reference count to zero.
2. The recycling code in msg_put does not test the reference count.
3. The allocation code does not remove the message from the pool,
   although this code was never reached because of point 2.

This patch fixes the issues and also adds some debugging code to trace
the message pool statistics.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 msg.c  | 39 ++++++++++++++++++++++++++++++++++++---
 port.c |  8 --------
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/msg.c b/msg.c
index c26c3e0..24bfe26 100644
--- a/msg.c
+++ b/msg.c
@@ -18,6 +18,7 @@
  */
 #include <arpa/inet.h>
 #include <malloc.h>
+#include <string.h>
 #include <time.h>
 
 #include <asm/byteorder.h>
@@ -40,6 +41,24 @@ struct message_storage {
 
 static TAILQ_HEAD(msg_pool, ptp_message) msg_pool;
 
+static struct {
+	int total;
+	int count;
+} pool_stats;
+
+#ifdef DEBUG_POOL
+static void pool_debug(const char *str, void *addr)
+{
+	fprintf(stderr, "*** %p %10s total %d count %d used %d\n",
+		addr, str, pool_stats.total, pool_stats.count,
+		pool_stats.total - pool_stats.count);
+}
+#else
+static void pool_debug(const char *str, void *addr)
+{
+}
+#endif
+
 static void announce_pre_send(struct announce_msg *m)
 {
 	m->currentUtcOffset = htons(m->currentUtcOffset);
@@ -145,13 +164,24 @@ struct ptp_message *msg_allocate(void)
 {
 	struct message_storage *s;
 	struct ptp_message *m = TAILQ_FIRST(&msg_pool);
-	if (!m) {
+
+	if (m) {
+		TAILQ_REMOVE(&msg_pool, m, list);
+		pool_stats.count--;
+		pool_debug("dequeue", m);
+	} else {
 		s = malloc(sizeof(*s));
 		if (s) {
 			m = &s->msg;
-			m->refcnt = 1;
+			pool_stats.total++;
+			pool_debug("allocate", m);
 		}
 	}
+	if (m) {
+		memset(m, 0, sizeof(*m));
+		m->refcnt = 1;
+	}
+
 	return m;
 }
 
@@ -330,8 +360,11 @@ void msg_print(struct ptp_message *m, FILE *fp)
 void msg_put(struct ptp_message *m)
 {
 	m->refcnt--;
-	if (!m)
+	if (!m->refcnt) {
+		pool_stats.count++;
+		pool_debug("recycle", m);
 		TAILQ_INSERT_HEAD(&msg_pool, m, list);
+	}
 }
 
 int msg_sots_missing(struct ptp_message *m)
diff --git a/port.c b/port.c
index bf5738f..9d3a966 100644
--- a/port.c
+++ b/port.c
@@ -366,7 +366,6 @@ static int port_pdelay_request(struct port *p)
 	msg = msg_allocate();
 	if (!msg)
 		return -1;
-	memset(msg, 0, sizeof(*msg));
 
 	pdulen = sizeof(struct pdelay_req_msg);
 	msg->hwts.type = p->timestamping;
@@ -414,7 +413,6 @@ static int port_delay_request(struct port *p)
 	msg = msg_allocate();
 	if (!msg)
 		return -1;
-	memset(msg, 0, sizeof(*msg));
 
 	pdulen = sizeof(struct delay_req_msg);
 	msg->hwts.type = p->timestamping;
@@ -461,7 +459,6 @@ static int port_tx_announce(struct port *p)
 	msg = msg_allocate();
 	if (!msg)
 		return -1;
-	memset(msg, 0, sizeof(*msg));
 
 	pdulen = sizeof(struct announce_msg);
 	msg->hwts.type = p->timestamping;
@@ -523,8 +520,6 @@ static int port_tx_sync(struct port *p)
 		msg_put(msg);
 		return -1;
 	}
-	memset(msg, 0, sizeof(*msg));
-	memset(fup, 0, sizeof(*fup));
 
 	pdulen = sizeof(struct sync_msg);
 	msg->hwts.type = p->timestamping;
@@ -767,7 +762,6 @@ static int process_delay_req(struct port *p, struct ptp_message *m)
 	msg = msg_allocate();
 	if (!msg)
 		return -1;
-	memset(msg, 0, sizeof(*msg));
 
 	pdulen = sizeof(struct delay_resp_msg);
 	msg->hwts.type = p->timestamping;
@@ -895,8 +889,6 @@ static int process_pdelay_req(struct port *p, struct ptp_message *m)
 		msg_put(rsp);
 		return -1;
 	}
-	memset(rsp, 0, sizeof(*rsp));
-	memset(fup, 0, sizeof(*fup));
 
 	rsp_len = sizeof(struct pdelay_resp_msg);
 	rsp->hwts.type = p->timestamping;