<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/4033/">https://reviewboard.asterisk.org/r/4033/</a>
     </td>
    </tr>
   </table>
   <br />



 <p>Ship it!</p>



 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">My "Ship it!" is a cautious one. Basically, the unit tests are good evidence to me that things are working as expected, but when changes are being made to such a fundamental area of code as the config API, I feel like a second "ship it!" from someone else should occur before actually committing.

Also, I think a discussion needs to occur with regards to this patch's inclusion in Asterisk 12.</pre>
 <br />







<div>




<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
 <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
    <a href="https://reviewboard.asterisk.org/r/4033/diff/3/?file=67794#file67794line256" style="color: black; font-weight: bold; text-decoration: underline;">branches/12/tests/test_config.c</a>
    <span style="font-weight: normal;">

     (Diff revision 3)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
    <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">256</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb">   </span><span class="tb">  </span><span class="n">snprintf</span><span class="p">(</span><span class="n">temp</span><span class="p">,</span> <span class="mi">32</span><span class="p">,</span> <span class="s">"test%d"</span><span class="p">,</span> <span class="n">i</span><span class="p">);</span></pre></td>
  </tr>

 </tbody>

</table>

<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">For all of the snprintf() calls in this test, use sizeof(temp) instead of 32.</pre>
</div>
<br />



<p>- Mark Michelson</p>


<br />
<p>On September 30th, 2014, 9:24 p.m. UTC, George Joseph wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Asterisk Developers.</div>
<div>By George Joseph.</div>


<p style="color: grey;"><i>Updated Sept. 30, 2014, 9:24 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch provides the capability to manipulate templates and categories with non-unique names via AMI.

Summary of changes:

GetConfig and GetConfigJSON:
Added "Filter" parameter:  A comma separated list of name_regex=value_regex expressions which will cause only categories whose variables match all expressions to be considered.  The special variable name TEMPLATES can be used to control whether templates are included.  Passing 'include' as the value will include templates along with normal categories. Passing 'restrict' as the value will restrict the operation to ONLY templates.  Not specifying a TEMPLATES expression results in the current default behavior which is to not include templates.

Examples:
"GetConfig?Filename=pjsip.conf&Category=myitsp&Filter=type=aor" would return only 'myitsp' categories with a type of 'aor'.

"GetConfig?Filename=pjsip.conf&Category=itsp-template&Filter=TEMPLATES=restrict,type=aor" would return only 'itsp-template' categories that are actually templates with a type of 'aor'.

"GetConfig?Filename=pjsip.conf&Filter=type=registration,endpoint=myitsp" would return only registrations whose corresponding endpoint is 'myitsp'.

The output from GetConfig and GetConfigJSON also includes variables indicating if the returned category is a template and the template names a category inherits from if any.

UpdateConfig:
NewCat now includes options for allowing duplicate category names, indicating if the category should be created as a template, and specifying templates the category should inherit from.  The rest of the actions now accept a filter string as defined above.  If there are non-unique category names, you can now update specific ones based on variable values.

To facilitate the new capabilities in manager, corresponding changes had to be made to config, most notably the addition of filter criteria to many of the APIs.  In some cases it was easy to change the references to use the new prototype but others would have required touching too many files for this patch so a wrapper with the original prototype was created.  Macros couldn't be used in this case because it would break binary compatibility with modules such as res_digium_phone that are linked to real symbols.

</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Testsuite before and after runs gave the same results.
A set of unit tests for config was added to test_config.  They test basic, filtered and template operations.
</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>branches/12/tests/test_sorcery_realtime.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/tests/test_sorcery.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/tests/test_config.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/res/res_sorcery_realtime.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/res/res_sorcery_config.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/pbx/pbx_realtime.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/main/manager.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/main/config.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/include/asterisk/config.h <span style="color: grey">(424175)</span></li>

 <li>branches/12/apps/app_voicemail.c <span style="color: grey">(424175)</span></li>

 <li>branches/12/apps/app_directory.c <span style="color: grey">(424175)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/4033/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>