<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/3607/">https://reviewboard.asterisk.org/r/3607/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 17th, 2014, 4:25 p.m. CDT, <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/3607/diff/1/?file=59533#file59533line8490" style="color: black; font-weight: bold; text-decoration: underline;">trunk/apps/app_queue.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int reload_queue_rules(int reload)</pre></td>
</tr>
</tbody>
<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">8490</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="tb"> </span>if (!(timestr) || (penaltychangetime = atoi(timestr)) < 0) {</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">atoi() does not detect errors (such as the input not being an integer) and will just return 0 on bad values. You can change this to:
if (!timestr || sscanf(timestr, "%30d", &penaltychangetime) != 1)
This will perform a more type-safe method of getting the value. The %30 in the format specifier is there as (an admittedly arbitrary) a way of preventing input of overly large numbers.</pre>
</blockquote>
<p>On June 23rd, 2014, 4:36 a.m. CDT, <b>Michael K.</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I tried to stick to the code used in parsing the rules from file.
But would be good idea to change it.
If we are on this topic, why not strtol()? or there is no reason and it's preferences.
Thanks.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, it's not arbitrary. From the Coding Guidelines [1]:
{quote}
2.15. String conversions
When converting from strings to integers or floats, use the sscanf function in preference to the atoi and atof family of functions, as sscanf detects errors. Always check the return value of sscanf to verify that your numeric variables successfully scanned before using them. Also, to avoid a potential libc bug, always specify a maximum width for each conversion specifier, including integers and floats. A good length for both integers and floats is 30, as this is more than generous, even if you're using doubles or long integers.
{quote}
For both atoi and strtol, you cannot tell a parsing error condition from the return value, as both return 0 on error. Generally, that's not good.
In the case of strtol, correct handling should check for overflow (ERANGE + LONG_MAX/LONG_MIN). This is cumbersome, as again, both max/min values are also valid values.
In the end, it's easier to use sscanf and specify the allowed width.
[1] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines</pre>
<br />
<p>- Matt</p>
<br />
<p>On June 23rd, 2014, 4:41 a.m. CDT, Michael K. 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 Michael K..</div>
<p style="color: grey;"><i>Updated June 23, 2014, 4:41 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23823">ASTERISK-23823</a>
</div>
<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 gives the ability (optional) to keep queuerules in realtime (important to notice that rules would be loaded from both realtime and file too)
To use add to queuerules.conf general section with option to turn the relatime on (note that after patch there is no option to call rule "general" as it is reserved for queuerules settings):
[general]
realtime_rules = yes
and add the realtime setting to extconfig.conf, for example in my case it's mysql and it looks like(will attach the .sql with create for table):
queue_rules => mysql,asterisk,queue_rules
in table as you can see you need to specify rule name and as in regular rules you can use relative setings for min and max (with "-" and "+", e.g. "+100")
if one of the rule parameters is wrong it would be ignored, basically validation is like from file.
here is the the example of table context as an example:
rule_name, time, min_penalty, max_penalty
'default', '10', '20', '30'
'test2', '20', '30', '55'
'test2', '25', '-11', '+1111'
'test2', '400', '112', '333'
'test3', '0', '4564', '46546'
'test_rule', '40', '15', '50'
which would result in :
Rule: default
After 10 seconds, adjust QUEUE_MAX_PENALTY to 30 and adjust QUEUE_MIN_PENALTY to 20
Rule: test2
After 20 seconds, adjust QUEUE_MAX_PENALTY to 55 and adjust QUEUE_MIN_PENALTY to 30
After 25 seconds, adjust QUEUE_MAX_PENALTY by 1111 and adjust QUEUE_MIN_PENALTY by -11
After 400 seconds, adjust QUEUE_MAX_PENALTY to 333 and adjust QUEUE_MIN_PENALTY to 112
Rule: test3
After 0 seconds, adjust QUEUE_MAX_PENALTY to 46546 and adjust QUEUE_MIN_PENALTY to 4564
Rule: test_rule
After 40 seconds, adjust QUEUE_MAX_PENALTY to 50 and adjust QUEUE_MIN_PENALTY to 15
If you use realtime, the rules will be always reloaded (no cache option here), in other words it would delete all rules and re-add them from realtime and file. It's important to understand that with realtime on it would reload rules from file doesn't matter if file was or was not change.
While with the option off it would act exactly like it was before patch (file would not be reloaded if it was not changed)</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;">Currently the tests were made on development machine.</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>trunk/apps/app_queue.c <span style="color: grey">(415442)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3607/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>