<p> Attention is currently required from: George Joseph. </p>
<p>Patch set 5:<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/+/18067">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/xml.h:</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/+/18067/comment/d5e5f16c_11c96973">Patch Set #5, Line 205:</a> <code style="font-family:monospace,monospace">anm</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Looks like this got missed s/anm/an</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File main/config.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/+/18067/comment/0e1e92b7_f0e9d249">Patch Set #5, Line 706:</a> <code style="font-family:monospace,monospace">               local_str = ast_str_create(AST_MAX_USER_FIELD);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Need to check for NULL here on local_str just in case</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/test_config.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/+/18067/comment/0a09a65f_eb23f012">Patch Set #5, Line 1900:</a> <code style="font-family:monospace,monospace">        struct ast_variable *var;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think this should be a const. See why below.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18067/comment/5407d2a8_af0851fc">Patch Set #5, Line 1936:</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;">        var = (struct ast_variable *)ast_variable_find_variable_in_list(list, "bbb");<br>       ast_test_validate(test, var != NULL);<br> ast_variable_list_replace_variable(&list, var, ast_variable_new("eee", "555", ""));<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Personally I think the cast should be done on ast_variable_list_replace_variable, so it's obvious why constness is being cast away.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Alternatively you could make the "old" parameter on ast_variable_list_replace_variable itself be a const as the free is done on a non const pointer internal to the function. However, that slightly hides the fact the memory pointed to by the "old" pointer is itself freed as it points to the same memory. Not sure if that makes any more sense.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18067">change 18067</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/+/18067"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: development/16/geolocation </div>
<div style="display:none"> Gerrit-Change-Id: I330a5f63dc0c218e0d8dfc0745948d2812141ccb </div>
<div style="display:none"> Gerrit-Change-Number: 18067 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.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: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 22 Feb 2022 21:38: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>