[asterisk-dev] [Code Review] Fix range checking with ast_parse_arg numeric types and add tests

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1879/#review6049
-----------------------------------------------------------



/branches/1.8/main/config.c
<https://reviewboard.asterisk.org/r/1879/#comment11084>

    You might want to add this to the PARSE_INT32 and PARSE_DOUBLE cases as well.



/branches/1.8/main/config.c
<https://reviewboard.asterisk.org/r/1879/#comment11086>

    Is this necessary? Does strtod not clear errno out?



/branches/1.8/tests/test_config.c
<https://reviewboard.asterisk.org/r/1879/#comment11085>

    It is typically considered bad form to check doubles for equality due to minute differences in representing the "same" value. I know the problem usually arises when attempting to compare calculated values as opposed to constants, but I still would feel better about changing this to check if the value falls within some tolerance instead of doing direct equality. I just fear that what works on your system may fail on others.


- Mark


On April 24, 2012, 10:11 a.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1879/
> -----------------------------------------------------------
> 
> (Updated April 24, 2012, 10:11 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I started out by writing unit tests for ast_parse_arg and found out that it didn't actually catch errors where the string parsed resulted in an overflow of the type. So this patch adds the tests and fixes the errors that the test found. I started writing tests for the PARSE_ADDR type, but it really is just a very simple wrapper around ast_sockaddr_parse() which is tested in the netsock2 tests. Since it didn't really seem like there was a benefit to testing it twice, I didn't include tests for that.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/config.c 362634 
>   /branches/1.8/tests/test_config.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1879/diff
> 
> 
> Testing
> -------
> 
> Wrote a test that now passes.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120424/edadfbe7/attachment.htm>