<p style="white-space: pre-wrap; word-wrap: break-word;">(would be nice to also see the code that is using this new IE in the same patch, or mention in the commit log the change id of the patch that will add that later)</p><p>Patch set 2:<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/c/osmo-msc/+/18692">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/osmo-msc/+/18692/2/include/osmocom/msc/ran_msg.h">File include/osmocom/msc/ran_msg.h:</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/osmo-msc/+/18692/2/include/osmocom/msc/ran_msg.h@89">Patch Set #2, Line 89:</a> <code style="font-family:monospace,monospace">       uint32_t call_id;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">each of these structs must allow indicating presence or absence of optional IEs.<br>The earlier patch set using call_id_present was exactly the right thing to do.<br>sorry to add another iteration, but please bring back call_id_present.</p><p style="white-space: pre-wrap; word-wrap: break-word;">(The idea of these structs is to represent 1:1 what the encoded message contained/should contain, and not make any assumptions on how individual IEs interact.)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-msc/+/18692/2/src/libmsc/ran_msg_a.c">File src/libmsc/ran_msg_a.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/osmo-msc/+/18692/2/src/libmsc/ran_msg_a.c@998">Patch Set #2, Line 998:</a> <code style="font-family:monospace,monospace">                   call_id = &ac->call_id;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't understand why the call_id is related to RTP address. I prefer the earlier patch set handling the call_id separately by call_id_present.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-msc/+/18692">change 18692</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/osmo-msc/+/18692"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4288f47e4a6d61ec672f431723f6e72c7c6b0799 </div>
<div style="display:none"> Gerrit-Change-Number: 18692 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </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: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Sun, 14 Jun 2020 12:10:46 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>