[asterisk-dev] [Code Review] Remove need for registration strings in sip.conf

[David Vossel]

> On 2010-06-23 14:42:49, David Vossel wrote:
> > trunk/channels/chan_sip.c, lines 4461-4488
> > <https://reviewboard.asterisk.org/r/718/diff/4/?file=11009#file11009line4461>
> >
> >     This is clever, but I don't think it will work the way you expect it to yet unless that peer is in the 'peers' ao2_container.   When 'the_mark' is set on a reload, only the peers in the 'peers' container are removed that have the_mark.  I don't think build_peer() puts the peer in that container.
> 
> Nick Lewis wrote:
>     This is a bit new and distant for me as I do not yet have any realtime test setup so may I clarify your comment. Are you saying that the code will not result in sip_registry items being created for realtime peers with register=yes or that the flushing out of the realtime peers will not happen just by setting the_mark? 
>     
>     My assumption at realtime operation was that when an incoming sip request was received a match was made on a peer definition in the database and it was loaded into memory. Once the relevant processing had taken place it was given the_mark and subject to garbage collection to free the memory. Can you please correct my understanding
> 
> Nick Lewis wrote:
>     Are you just saying that it needs:
>     
>      peer = build_peer(...);
>      if (peer) {
>       ao2_t_link(peers, peer, ...);
>       peer->the_mark = 1;
>      }
>     
>     because build_peer omits to link it when it allocates the ao2 memory for the peer?
>     
>     What happens if build_peer returns NULL because of a problem after allocating the ao2 memory?
> 
> Nick Lewis wrote:
>     Sorry - I now see that what happens is an unref_peer (to zero and free) before returning NULL - which got me thinking. Rather than using the_mark is there anything to stop me just doing an unref_peer() to free the allocation after build_peer. The sip_registry object requires no references to the peer object that triggered its creation so the peer object could I guess be destroyed immediately:
>     
>      peer = build_peer(...);
>      if (peer) {
>       unref_peer(peer,...);
>      }
>

Ha, that is exactly what you should do, Just unref it.  I don't know why I didn't think of that to begin with.


- David


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


On 2010-06-23 11:03:32, Nick Lewis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/718/
> -----------------------------------------------------------
> 
> (Updated 2010-06-23 11:03:32)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> This change adds the ability to specify the sip registration expiry and registrar host to the peer configuration items. This permits all registration details that are available to the registration string to be also available to the peer definitions. The users of sip.conf can enable registration by setting register=yes in the peer and do not need to concern themselves with the strange format of the registration strings
> 
> Suggest inclusion in 1.8
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_sip.c 268968 
>   trunk/channels/sip/config_parser.c 268968 
>   trunk/channels/sip/include/sip.h 268968 
>   trunk/configs/sip.conf.sample 268968 
> 
> Diff: https://reviewboard.asterisk.org/r/718/diff
> 
> 
> Testing
> -------
> 
> Compile, run and confirm in cli sip show registry
> 
> 
> Thanks,
> 
> Nick
> 
>