[asterisk-dev] [Code Review] Parse mwi => lines in sip.conf properly to avoid segfaults and have it work.

Russell Bryant reviewboard at asterisk.org
Thu Dec 16 12:55:33 UTC 2010



> On 2010-12-16 06:52:43, Russell Bryant wrote:
> > Nice work!  Thanks for adding a unit test!
> > 
> > My comments are all very minor, so feel free to commit after making these tweaks.  My only other comment, which is just an idea for a potential future enhancement to the test, would be to add some bogus input strings that you expect to fail to make sure that they are handled gracefully.

On the topic of expected failures, what about adding test input for a line that doesn't contain '@'?  It doesn't look like the code was actually changed based on dvossel's earlier review.


- Russell


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


On 2010-12-15 17:17:49, Marquis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1053/
> -----------------------------------------------------------
> 
> (Updated 2010-12-15 17:17:49)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Modify the parsing of mwi subscription lines in sip.conf in order to avoid an existing segfault and to have it properly populate the struct for subscriptions.
> 
> 
> This addresses bug 18350.
>     https://issues.asterisk.org/view.php?id=18350
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 298391 
>   /branches/1.8/configs/sip.conf.sample 298391 
> 
> Diff: https://reviewboard.asterisk.org/r/1053/diff
> 
> 
> Testing
> -------
> 
> Added lines for various possible permutations (kept in the diff for documentation) as well as temporary (not in the diff) debug lines to verify all strings were set to their expected values.  Reporter in issue 18350 also applied patch and verified that his segfault was gone and that subscriptions were working.
> 
> 
> Thanks,
> 
> Marquis
> 
>

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


More information about the asterisk-dev mailing list