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



/trunk/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/4515/#comment25337>

    This could result in a change of behaviour.  In build_peer 'first_pass' 
will no longer decide if the existing mailboxes get cleared, it would only be 
done when '!dev_state'.
    
    I feel that a safer change would be to just remove the delme variable, let 
the current logic live on.  I could be convinced otherwise if you can show that 
the current logic produces the wrong results, but even then we'd have to very 
careful.


- Corey Farrell


On March 19, 2015, 11:02 p.m., gareth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4515/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 11:02 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24871
>     https://issues.asterisk.org/jira/browse/ASTERISK-24871
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> During a reload, build_peer iterates over the peer's mailboxes and tags them 
> for removal via the delme variable. It adds any new, unique mailboxes to the 
> peer via add_peer_mailboxes and then removes any mailboxes with delme still 
> set.
> 
> However, there isn't any code to unset delme, so this would remove any 
> previously configured mailboxes.
> 
> That is not what happens though because build_peer also calls 
> set_peer_defaults which clears out all of the configured mailboxes using 
> clear_peer_mailboxes making the setting of delme redundant.
> 
> So in the end there is no impact to the user because all the configured 
> mailboxes get added regardless.
> 
> Patch unsets delme for existing, still-configured mailboxes in 
> add_peer_mailboxes and removes call to clear_peer_mailboxes.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 433198 
> 
> Diff: https://reviewboard.asterisk.org/r/4515/diff/
> 
> 
> Testing
> -------
> 
> Added new mailboxes to peer, reloaded chan_sip and verified that existing 
> mailboxes were still there and new mailboxes had been added.
> 
> Removed mailboxes from peer, reloaded chan_sip and verified that those 
> mailboxes were no longer assigned to peer.
> 
> 
> Thanks,
> 
> gareth
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to