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

[Terry Wilson]

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

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 358434 
  /branches/1.8/include/asterisk/devicestate.h 358434 
  /branches/1.8/main/devicestate.c 358434 

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/20120308/01f8aa66/attachment.htm>