> On June 17, 2014, 9:25 p.m., Mark Michelson wrote: > > trunk/apps/app_queue.c, line 8490 > > <https://reviewboard.asterisk.org/r/3607/diff/1/?file=59533#file59533line8490> > > > > 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. > > Michael K. wrote: > 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. > > Matt Jordan wrote: > 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
thanks for explanation :) - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3607/#review12179 ----------------------------------------------------------- On June 23, 2014, 9:41 a.m., Michael K. wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3607/ > ----------------------------------------------------------- > > (Updated June 23, 2014, 9:41 a.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23823 > https://issues.asterisk.org/jira/browse/ASTERISK-23823 > > > Repository: Asterisk > > > Description > ------- > > 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) > > > Diffs > ----- > > trunk/apps/app_queue.c 415442 > > Diff: https://reviewboard.asterisk.org/r/3607/diff/ > > > Testing > ------- > > Currently the tests were made on development machine. > > > Thanks, > > Michael K. > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
