<p style="white-space: pre-wrap; word-wrap: break-word;">@corey, Thanks. :) I've fixed it all.</p><p><a href="https://gerrit.asterisk.org/10929">View Change</a></p><p>9 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c">File main/stasis.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/10929/4/main/stasis.c@308">Patch Set #4, Line 308:</a> <code style="font-family:monospace,monospace">#if defined(LOW_MEMORY)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">We might want an `#if defined(LOW_MEMORY)` block with a smaller number of buckets.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, I've fixed it. :)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@376">Patch Set #4, Line 376:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You do not need the buffer in 'struct stasis_topic'. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Thanks, I've fixed it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@384">Patch Set #4, Line 384:</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;">struct ao2_container *topic_all;<br><br>struct topic_proxy {<br> AO2_WEAKPROXY();<br><br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">These pointers should not be on the weakproxy. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Thanks, I've fixed it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@459">Patch Set #4, Line 459:</a> <code style="font-family:monospace,monospace"> if (!topic || !name || !detail) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">After locking the container you should probably check for an existing topic with `name`, abort with […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Sure, fixed it. :)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@461">Patch Set #4, Line 461:</a> <code style="font-family:monospace,monospace"> }</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The allocation needs to include space for name and detail to be stored in buf.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, fixed it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@463">Patch Set #4, Line 463:</a> <code style="font-family:monospace,monospace"> ao2_wrlock(topic_all);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Nit: could you `return -1` for errors? […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">No problem.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Return -1 when it's an error. And added unlock as well.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@466">Patch Set #4, Line 466:</a> <code style="font-family:monospace,monospace"> if (topic_tmp) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">s/sorcery/topic/ […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Sure, fixed it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@477">Patch Set #4, Line 477:</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;"><br> ao2_unlock(topic_all);<br> return -1;<br> }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Please remove this. proxy can't point to memory owned by topic.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Sure, fixed it. :)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10929/4/main/stasis.c@2232">Patch Set #4, Line 2232:</a> <code style="font-family:monospace,monospace">#undef FMT_HEADERS</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Please use ast_cli_completion_add instead of returning the result. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, fixed it! :)</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/10929">change 10929</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/10929"/><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: Ie86d125d2966f93de74ee00f47ae6fbc8c081c5f </div>
<div style="display:none"> Gerrit-Change-Number: 10929 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation (1000185) </div>
<div style="display:none"> Gerrit-Reviewer: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 28 Jan 2019 13:43:44 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>