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

[Terry Wilson]

> On April 24, 2012, 3:26 p.m., Mark Michelson wrote:
> > /branches/1.8/main/config.c, line 2726
> > <https://reviewboard.asterisk.org/r/1879/diff/1/?file=27527#file27527line2726>
> >
> >     Is this necessary? Does strtod not clear errno out?

The example at http://www.kernel.org/doc/man-pages/online/pages/man3/strtol.3.html#EXAMPLE specifically set errno=0, so I thought it might be a good idea to follow suit.


> On April 24, 2012, 3:26 p.m., Mark Michelson wrote:
> > /branches/1.8/main/config.c, line 2683
> > <https://reviewboard.asterisk.org/r/1879/diff/1/?file=27527#file27527line2683>
> >
> >     You might want to add this to the PARSE_INT32 and PARSE_DOUBLE cases as well.

strto* skips blanks itself, but since I needed to see if there was a leading '-', I had to do it myself.


> On April 24, 2012, 3:26 p.m., Mark Michelson wrote:
> > /branches/1.8/tests/test_config.c, lines 73-77
> > <https://reviewboard.asterisk.org/r/1879/diff/1/?file=27528#file27528line73>
> >
> >     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.

Ok.


- Terry


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


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/84df4d14/attachment.htm>