<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/11993">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11993/1//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11993/1//COMMIT_MSG@23">Patch Set #1, Line 23:</a> <code style="font-family:monospace,monospace">20181128193707330 DAS <0012> fsm.c:381 assignment(conn4_0-0-6-TCH_F_PDCHasPDCH-0)[0x6120000024a0]{WAIT_LCHAN_ACTIVE}: Deallocated</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">you *could* make this a bit more readable next time :)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11993/1/src/osmo-bsc/lchan_rtp_fsm.c">File src/osmo-bsc/lchan_rtp_fsm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11993/1/src/osmo-bsc/lchan_rtp_fsm.c@a742">Patch Set #1, Line 742:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">while it is true that this is normally a duplicate event, it does make sure that in case the RTP hits a cleanup for any odd reason, we will notify the lchan of it.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Say something went wrong during MGCP or RTP delivery, or the lchan_rtp fi gets terminated out of the blue... (If the lchan_rtp FSM gets deallocated, the lchan must be *guaranteed* to no longer reference the lchan_rtp fi pointer.)</p><p style="white-space: pre-wrap; word-wrap: break-word;">Maybe rather something like make this conditional on the current lchan state, or some flag that tells us whether we've already told the lchan that RTP was released? Or see whether the lchan still points at this fi?</p><p style="white-space: pre-wrap; word-wrap: break-word;">Simplest is to just keep the duplicate event? It's not harmful...</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/11993">change 11993</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/11993"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I3e95a21e5a5ec6c35b1ab20b7a642fd7eb81e556 </div>
<div style="display:none"> Gerrit-Change-Number: 11993 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 29 Nov 2018 14:34:11 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>