<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8773">View Change</a></p><p>Patch set 3:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p style="white-space: pre-wrap; word-wrap: break-word;">Minor nits</p><p>(2 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8773/3/res/res_rtp_asterisk.c">File res/res_rtp_asterisk.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8773/3/res/res_rtp_asterisk.c@4413">Patch Set #3, Line 4413:</a> <code style="font-family:monospace,monospace">                  if ((payload = ast_calloc(1, sizeof(*payload) + packet_len))) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Use ast_malloc() instead since you are immediately setting all members in payload manually so there is no need for ast_calloc() to set them to zero first.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also you should avoid doing assignments in if statements.  There are better line break opportunities when it is done as an assignment followed by an allocation check.  It is also easier to read the code.</p><p style="white-space: pre-wrap; word-wrap: break-word;">payload = ast_malloc(sizeof(*payload) + packet_len);<br>if (payload) {<br>}</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8773/3/res/res_rtp_asterisk.c@4420">Patch Set #3, Line 4420:</a> <code style="font-family:monospace,monospace">           if ((res = rtp_sendto(instance, (void *)rtpheader, packet_len, 0, &remote_address, &ice)) < 0) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Might as well pull the assignment out here so the line isn't as long since you are touching the line:</p><p style="white-space: pre-wrap; word-wrap: break-word;">res = rtp_sendto(instance...);<br>if (res < 0) {<br>}</p><p style="white-space: pre-wrap; word-wrap: break-word;">Much easier to read and you can easily see what the if test is checking now.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8773">change 8773</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/8773"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I7f7f124af3b9d5d2fd9cffc6ba8cb48a6fff06ec </div>
<div style="display:none"> Gerrit-Change-Number: 8773 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@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: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 16 Apr 2018 21:15:35 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>