i460_mux: correctly reset subchannels

When a subchannel is deleted or created the initalization mainly
consists of a memset over the wohle subchannel struct a message buffer
initailization.

However, when we delete a subchannel we also must take of the resetting
of the related struct. Currently this is done with a memest.
Unfortunately this creates not only a memory leak (there might be still
items in the multiplexer tx queue) but also it makes the application
crash when the message buffer is used the next time since the llist_head
of the tx queue looses its initialization.

Lets fix the memory leak problem and the message buffer problem and put
the reset functionality in a single place.

Change-Id: I937a9d4db95f44a860cd2c5dbb660dc1970b9a49

1 files changed, 23 insertions, 7 deletions

diff --git a/src/gsm/i460_mux.c b/src/gsm/i460_mux.c
index 3fb63ec0..50cb56e9 100644
--- a/src/gsm/i460_mux.c
+++ b/src/gsm/i460_mux.c
@@ -306,6 +306,26 @@ static int find_unused_subchan_idx(const struct osmo_i460_timeslot *ts)
 	return -1;
 }
 
+/* reset subchannel struct into a defined state */
+static void subchan_reset(struct osmo_i460_subchan *schan, bool first_time)
+{
+	/* Before we zero out the subchannel struct, we must be sure that the
+	 * tx_queue is cleared and all dynamically allocated memory is freed.
+	 * However, on an uninitalized subchannel struct we can not be sure
+	 * that the pointers are valid. If the subchannel is reset the first
+	 * time the caller must set first_time to true. */
+	if (!first_time) {
+		if (schan->demux.out_bitbuf)
+			talloc_free(schan->demux.out_bitbuf);
+		msgb_queue_free(&schan->mux.tx_queue);
+	}
+
+	/* Reset subchannel to a defined state */
+	memset(schan, 0, sizeof(*schan));
+	schan->rate = OSMO_I460_RATE_NONE;
+	INIT_LLIST_HEAD(&schan->mux.tx_queue);
+}
+
 /*! initialize an I.460 timeslot */
 void osmo_i460_ts_init(struct osmo_i460_timeslot *ts)
 {
@@ -313,10 +333,7 @@ void osmo_i460_ts_init(struct osmo_i460_timeslot *ts)
 
 	for (i = 0; i < ARRAY_SIZE(ts->schan); i++) {
 		struct osmo_i460_subchan *schan = &ts->schan[i];
-
-		memset(schan, 0, sizeof(*schan));
-		schan->rate = OSMO_I460_RATE_NONE;
-		INIT_LLIST_HEAD(&schan->mux.tx_queue);
+		subchan_reset(schan, true);
 	}
 }
 
@@ -345,7 +362,7 @@ osmo_i460_subchan_add(void *ctx, struct osmo_i460_timeslot *ts, const struct osm
 	schan->demux.user_data = chd->demux.user_data;
 	rc = alloc_bitbuf(ctx, schan, chd->demux.num_bits);
 	if (rc < 0) {
-		memset(schan, 0, sizeof(*schan));
+		subchan_reset(schan, false);
 		return NULL;
 	}
 
@@ -356,8 +373,7 @@ osmo_i460_subchan_add(void *ctx, struct osmo_i460_timeslot *ts, const struct osm
 /* remove a su-channel from the multiplex */
 void osmo_i460_subchan_del(struct osmo_i460_subchan *schan)
 {
-	talloc_free(schan->demux.out_bitbuf);
-	memset(schan, 0, sizeof(*schan));
+	subchan_reset(schan, false);
 }
 
 /*! @} */