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

[Marquis]

> On 2010-12-09 09:59:09, David Vossel wrote:
> > /branches/1.8/channels/chan_sip.c, lines 7581-7594
> > <https://reviewboard.asterisk.org/r/1053/diff/1/?file=14445#file14445line7581>
> >
> >     if no '@' character is present in buf, username will remain NULL.  Doing a strchr on a NULL pointer will crash I believe.
> >     
> >     Are you familiar with our ability to perform unit tests?  Parsing functions like this are great areas to make unit tests.

Oh boy, nice mistake there... :/
In fixing one crash bug I precisely replicated the exact kind of bug. :)

Also, I hear your and Russell's comments about a unit test.  Good call, and I'll go figure out how that works and update this review.


- Marquis


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


On 2010-12-09 08:49:25, Marquis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1053/
> -----------------------------------------------------------
> 
> (Updated 2010-12-09 08:49:25)
> 
> 
> 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 297775 
>   /branches/1.8/configs/sip.conf.sample 297775 
> 
> 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/20101209/bdcee59c/attachment.htm