<p>Joshua Colp <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8379">View Change</a></p><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>(1 comment)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8379/1/res/res_pjsip_session.c">File res/res_pjsip_session.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8379/1/res/res_pjsip_session.c@895">Patch Set #1, Line 895:</a> <code style="font-family:monospace,monospace">           media_state = session->active_media_state;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm not comfortable with this because the subsequent code assumes that the media state is not immutable. When a media state is made active it becomes immutable, so that violates the contract. While it is possible to do this if it is not kept in mind in the future for future changes then crashes can occur.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think instead of directly using active_media_state it should be cloned into pending_media_state using ast_sip_session_media_state_clone if pending_media_state is not present. This ensures that the contract remains.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8379">change 8379</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.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.asterisk.org/8379"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: If0d5150ffe6f38d8a854831fef37942258d4629c </div>
<div style="display:none"> Gerrit-Change-Number: 8379 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: lvl <digium@lvlconsultancy.nl> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 05 Mar 2018 13:37:01 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>