<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19381">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">i460_mux: correctly reset subchannels<br><br>When a subchannel is deleted or created the initalization mainly<br>consists of a memset over the wohle subchannel struct a message buffer<br>initailization.<br><br>However, when we delete a subchannel we also must take of the resetting<br>of the related struct. Currently this is done with a memest.<br>Unfortunately this creates not only a memory leak (there might be still<br>items in the multiplexer tx queue) but also it makes the application<br>crash when the message buffer is used the next time since the llist_head<br>of the tx queue looses its initialization.<br><br>Lets fix the memory leak problem and the message buffer problem and put<br>the reset functionality in a single place.<br><br>Change-Id: I937a9d4db95f44a860cd2c5dbb660dc1970b9a49<br>---<br>M src/gsm/i460_mux.c<br>1 file changed, 23 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gsm/i460_mux.c b/src/gsm/i460_mux.c</span><br><span>index 3fb63ec..50cb56e 100644</span><br><span>--- a/src/gsm/i460_mux.c</span><br><span>+++ b/src/gsm/i460_mux.c</span><br><span>@@ -306,6 +306,26 @@</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* reset subchannel struct into a defined state */</span><br><span style="color: hsl(120, 100%, 40%);">+static void subchan_reset(struct osmo_i460_subchan *schan, bool first_time)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Before we zero out the subchannel struct, we must be sure that the</span><br><span style="color: hsl(120, 100%, 40%);">+ * tx_queue is cleared and all dynamically allocated memory is freed.</span><br><span style="color: hsl(120, 100%, 40%);">+ * However, on an uninitalized subchannel struct we can not be sure</span><br><span style="color: hsl(120, 100%, 40%);">+ * that the pointers are valid. If the subchannel is reset the first</span><br><span style="color: hsl(120, 100%, 40%);">+ * time the caller must set first_time to true. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!first_time) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (schan->demux.out_bitbuf)</span><br><span style="color: hsl(120, 100%, 40%);">+ talloc_free(schan->demux.out_bitbuf);</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_queue_free(&schan->mux.tx_queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Reset subchannel to a defined state */</span><br><span style="color: hsl(120, 100%, 40%);">+ memset(schan, 0, sizeof(*schan));</span><br><span style="color: hsl(120, 100%, 40%);">+ schan->rate = OSMO_I460_RATE_NONE;</span><br><span style="color: hsl(120, 100%, 40%);">+ INIT_LLIST_HEAD(&schan->mux.tx_queue);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! initialize an I.460 timeslot */</span><br><span> void osmo_i460_ts_init(struct osmo_i460_timeslot *ts)</span><br><span> {</span><br><span>@@ -313,10 +333,7 @@</span><br><span> </span><br><span> for (i = 0; i < ARRAY_SIZE(ts->schan); i++) {</span><br><span> struct osmo_i460_subchan *schan = &ts->schan[i];</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- memset(schan, 0, sizeof(*schan));</span><br><span style="color: hsl(0, 100%, 40%);">- schan->rate = OSMO_I460_RATE_NONE;</span><br><span style="color: hsl(0, 100%, 40%);">- INIT_LLIST_HEAD(&schan->mux.tx_queue);</span><br><span style="color: hsl(120, 100%, 40%);">+ subchan_reset(schan, true);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>@@ -345,7 +362,7 @@</span><br><span> schan->demux.user_data = chd->demux.user_data;</span><br><span> rc = alloc_bitbuf(ctx, schan, chd->demux.num_bits);</span><br><span> if (rc < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- memset(schan, 0, sizeof(*schan));</span><br><span style="color: hsl(120, 100%, 40%);">+ subchan_reset(schan, false);</span><br><span> return NULL;</span><br><span> }</span><br><span> </span><br><span>@@ -356,8 +373,7 @@</span><br><span> /* remove a su-channel from the multiplex */</span><br><span> void osmo_i460_subchan_del(struct osmo_i460_subchan *schan)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- talloc_free(schan->demux.out_bitbuf);</span><br><span style="color: hsl(0, 100%, 40%);">- memset(schan, 0, sizeof(*schan));</span><br><span style="color: hsl(120, 100%, 40%);">+ subchan_reset(schan, false);</span><br><span> }</span><br><span> </span><br><span> /*! @} */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/19381">change 19381</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/19381"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I937a9d4db95f44a860cd2c5dbb660dc1970b9a49 </div>
<div style="display:none"> Gerrit-Change-Number: 19381 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>