<p> Attention is currently required from: Michael Bradeen, Kevin Harwell. </p>
<p>Patch set 6:<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/+/16571">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</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/+/16571/comment/baae4ae9_ae9024a7">Patch Set #5, Line 12:</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;">test_linkedlist: 'bogus' variable was manually allocated from a macro, and the<br>test fails if this happens but the compiler couldn't 'see' this and returns a<br>warning. 'stackbogus' variable used instead for the warned test instead.<br>chan_ooh323: Fixed various indentation issues that triggered misleading indentation<br>warnings.<br><br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Long lines. Keep them under 72. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File addons/ooh323c/src/ooh323.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/+/16571/comment/0f4e0157_ea66f811">Patch Set #5, Line 1069:</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> if (call->h225version >= 4) {<br> ret =ooSendTCSandMSD(call);<br> }<br> if (ret != OO_OK) {<br> return ret;<br> }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">More mess (not your doing). […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/test_linkedlists.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/+/16571/comment/536655a1_478fa60f">Patch Set #6, Line 190:</a> <code style="font-family:monospace,monospace"> if (AST_LIST_REMOVE(&test_list, &stackbogus, list)) {</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">This is an odd one indeed. Do you know why it complains about this one, but not for the same call above (the one just after the alloca call)?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">After looking a little more at this I think the issue is that 'bogus' was never initialized with zeros. When the first call to AST_LIST_REMOVE is done there have been no operations on the list and I think the compiler is keeping track of that so the fact that bogus is, well, bogus, in that first call is ignored. As soon as you add an item to the list then try to traverse it (MATCH_OR_FAIL/list_expect) the compiler now cares and the next time you try to use bogus, it barfs.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Rather than adding stackbogus, I think the correct solution is to just initialize bogus right after the call to ast_alloca...<br>memset(bogus, 0, sizeof(*bogus));</p><p style="white-space: pre-wrap; word-wrap: break-word;">Compile and test work fine after that.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16571">change 16571</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/+/16571"/><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-Change-Id: If4fe42222c8444dc16828a42731ee53b4ce5cbbe </div>
<div style="display:none"> Gerrit-Change-Number: 16571 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </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: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-CC: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 20 Oct 2021 11:50:30 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Comment-In-Reply-To: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>