<p>Matthew Fredrickson <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8604">View Change</a></p><p>Patch set 4:</p><p>(3 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8604/4/main/data_buffer.c">File main/data_buffer.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8604/4/main/data_buffer.c@228">Patch Set #4, Line 228:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">               ast_data_buffer_cache_adjust(buffer);<br>         buffer_payload = data_buffer_payload_alloc(payload, pos);<br>             if (!buffer_payload) {<br>                        return -1;<br>            }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If you call the adjust function every time it will only adjust if the sizes</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm starting to think that the cache might not be useful as well.  If we're not ever retiring objects to the cache, it seems to have little point in existing other than for startup memory allocation wait time (or resize allocation wait time).</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8604/4/main/data_buffer.c@237">Patch Set #4, Line 237:</a> <code style="font-family:monospace,monospace">                if (existing_payload->pos > pos) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I suppose that dealing with RTP seqno wrapping will be something that is dealt with outside of this code?</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8604/4/main/data_buffer.c@260">Patch Set #4, Line 260:</a> <code style="font-family:monospace,monospace"> if (pos == -1) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Good catch. I was considering changing it to a 0 check, but I'm not sure if</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">NULL is 0 also.  0 will probably be a valid value in some cases, so you're correct in being concerned about using it.  You could add an additional function that grabs from the head of the list instead, if it's important enough.  I'd target our initial use case though - if we don't have a reason to flexibly grab both by pos and from the list head, I'd pick the one we're planning on using first.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8604">change 8604</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/8604"/><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: Iff13c5d4795d52356959fe2a57360cd57dfade07 </div>
<div style="display:none"> Gerrit-Change-Number: 8604 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: 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: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 21 Mar 2018 19:40:39 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>