[asterisk-dev] [Code Review] Make hints for non-existant SIP devices show up as Unavailable instead of Idle

[Mark Michelson]

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

Ship it!


- Mark


On March 12, 2012, 4:37 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1808/
> -----------------------------------------------------------
> 
> (Updated March 12, 2012, 4:37 p.m.)
> 
> 
> Review request for Asterisk Developers and schmidts.
> 
> 
> Summary
> -------
> 
> First, a big thanks to Elazar Broad and Stefan Schmidt for the detective work done in ASTERISK-16702 tracking down this issue. The solution presented here is almost directly from their discussion in the issue.
> 
> Several issues result in non-existant SIP devices showing up as Idle when doing 'core show hints'.
> 
> First, in chan_sip, there used to be a section of code in sip_devicestate() that tried to resolve a hostname, and if it was unresolvable return AST_DEVICE_UNKNOWN instead of the default AST_DEVICE_INVALID. When this resolving code was removed, AST_DEVICE_INVALID would never be returned--only AST_DEVICE_UNKOWN. AST_DEVICE_UNKNOWN means the device is valid, but we don't know it's state and is the wrong result for a non-existant device.
> 
> Second, ast_devstate_aggregate_result had no way of actually returning AST_DEVICE_INVALID even if it was the result.
> 
> Third, ast_destate_aggregate_add would return the same results for both AST_DEVICE_UNKNOWN and AST_DEVICE_INVALID, making it impossible to differentiate between the two in ast_devstate_aggregate_result.
> 
> This patch fixes the above issues. The patch is longer than it technically needs to be. The agg->all_invalid state can be removed and instead just return AST_DEVICE_INVALID for the agg->all_unavail && agg->all_unknown case in ast_devstate_aggregate_result (indeed, this would be exactly what Elazar suggests in the issue). But, I thought that more explicit behavior might be less prone to future bugs. Either method would work for me.
> 
> 
> This addresses bug ASTERISK-16702.
>     https://issues.asterisk.org/jira/browse/ASTERISK-16702
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 358806 
>   /branches/1.8/include/asterisk/devicestate.h 358806 
>   /branches/1.8/main/devicestate.c 358806 
>   /branches/1.8/tests/test_devicestate.c 358806 
> 
> Diff: https://reviewboard.asterisk.org/r/1808/diff
> 
> 
> Testing
> -------
> 
> Tested with unregistered, registered, and defined peers and combination of the three. Results look sane.
> 
> 
> Thanks,
> 
> Terry
> 
>

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