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>
master
Richard Cochran 2012-07-07 20:10:28 +02:00
parent b34f74bf03
commit 3d7372d529
2 changed files with 36 additions and 11 deletions

39
msg.c
View File

@ -18,6 +18,7 @@
*/ */
#include <arpa/inet.h> #include <arpa/inet.h>
#include <malloc.h> #include <malloc.h>
#include <string.h>
#include <time.h> #include <time.h>
#include <asm/byteorder.h> #include <asm/byteorder.h>
@ -40,6 +41,24 @@ struct message_storage {
static TAILQ_HEAD(msg_pool, ptp_message) msg_pool; 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) static void announce_pre_send(struct announce_msg *m)
{ {
m->currentUtcOffset = htons(m->currentUtcOffset); m->currentUtcOffset = htons(m->currentUtcOffset);
@ -145,13 +164,24 @@ struct ptp_message *msg_allocate(void)
{ {
struct message_storage *s; struct message_storage *s;
struct ptp_message *m = TAILQ_FIRST(&msg_pool); 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)); s = malloc(sizeof(*s));
if (s) { if (s) {
m = &s->msg; m = &s->msg;
pool_stats.total++;
pool_debug("allocate", m);
}
}
if (m) {
memset(m, 0, sizeof(*m));
m->refcnt = 1; m->refcnt = 1;
} }
}
return m; return m;
} }
@ -330,9 +360,12 @@ void msg_print(struct ptp_message *m, FILE *fp)
void msg_put(struct ptp_message *m) void msg_put(struct ptp_message *m)
{ {
m->refcnt--; m->refcnt--;
if (!m) if (!m->refcnt) {
pool_stats.count++;
pool_debug("recycle", m);
TAILQ_INSERT_HEAD(&msg_pool, m, list); TAILQ_INSERT_HEAD(&msg_pool, m, list);
} }
}
int msg_sots_missing(struct ptp_message *m) int msg_sots_missing(struct ptp_message *m)
{ {

8
port.c
View File

@ -366,7 +366,6 @@ static int port_pdelay_request(struct port *p)
msg = msg_allocate(); msg = msg_allocate();
if (!msg) if (!msg)
return -1; return -1;
memset(msg, 0, sizeof(*msg));
pdulen = sizeof(struct pdelay_req_msg); pdulen = sizeof(struct pdelay_req_msg);
msg->hwts.type = p->timestamping; msg->hwts.type = p->timestamping;
@ -414,7 +413,6 @@ static int port_delay_request(struct port *p)
msg = msg_allocate(); msg = msg_allocate();
if (!msg) if (!msg)
return -1; return -1;
memset(msg, 0, sizeof(*msg));
pdulen = sizeof(struct delay_req_msg); pdulen = sizeof(struct delay_req_msg);
msg->hwts.type = p->timestamping; msg->hwts.type = p->timestamping;
@ -461,7 +459,6 @@ static int port_tx_announce(struct port *p)
msg = msg_allocate(); msg = msg_allocate();
if (!msg) if (!msg)
return -1; return -1;
memset(msg, 0, sizeof(*msg));
pdulen = sizeof(struct announce_msg); pdulen = sizeof(struct announce_msg);
msg->hwts.type = p->timestamping; msg->hwts.type = p->timestamping;
@ -523,8 +520,6 @@ static int port_tx_sync(struct port *p)
msg_put(msg); msg_put(msg);
return -1; return -1;
} }
memset(msg, 0, sizeof(*msg));
memset(fup, 0, sizeof(*fup));
pdulen = sizeof(struct sync_msg); pdulen = sizeof(struct sync_msg);
msg->hwts.type = p->timestamping; msg->hwts.type = p->timestamping;
@ -767,7 +762,6 @@ static int process_delay_req(struct port *p, struct ptp_message *m)
msg = msg_allocate(); msg = msg_allocate();
if (!msg) if (!msg)
return -1; return -1;
memset(msg, 0, sizeof(*msg));
pdulen = sizeof(struct delay_resp_msg); pdulen = sizeof(struct delay_resp_msg);
msg->hwts.type = p->timestamping; msg->hwts.type = p->timestamping;
@ -895,8 +889,6 @@ static int process_pdelay_req(struct port *p, struct ptp_message *m)
msg_put(rsp); msg_put(rsp);
return -1; return -1;
} }
memset(rsp, 0, sizeof(*rsp));
memset(fup, 0, sizeof(*fup));
rsp_len = sizeof(struct pdelay_resp_msg); rsp_len = sizeof(struct pdelay_resp_msg);
rsp->hwts.type = p->timestamping; rsp->hwts.type = p->timestamping;