<p> Attention is currently required from: Michael Bradeen. </p>
<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/16637">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_rtp_asterisk.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.asterisk.org/c/asterisk/+/16637/comment/64fcd5cf_17e5cd92">Patch Set #1, Line 3865:</a> <code style="font-family:monospace,monospace">  x = (x == rtpstart && x != (x & ~1)) ? rtpstart + 1 : x;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">It takes a little bit of staring to figure out what's going on here, so I think adding a comment explaining things a bit would be good. Or make the code easier to read based on what the checks are doing. For instance might not be a bad idea to just add some IS_EVEN, and/or IS_ODD macros in utils.h or somewhere.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also I don't think the x == rtpstart check is required? What if 'x' is an odd number not equal to rtpstart?</p><p style="white-space: pre-wrap; word-wrap: break-word;">As well the x != (x & ~1)) check. I think (x & 1) alone will tell you if the value is even or odd so no need for the equality (x != ..) check?</p><p style="white-space: pre-wrap; word-wrap: break-word;">Lastly, and this might be being checked elsewhere, but is it possible for rtpstart to be greater than rtpend, or the variable startplace to be calculated to be? What happens if rtpstart = rptend - 1, and rtpstart is calculated to be an odd number?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16637">change 16637</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/16637"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I90f07deef0716da4a30206e9f849458b2dbe346b </div>
<div style="display:none"> Gerrit-Change-Number: 16637 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 04 Nov 2021 21:50:29 +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>