[asterisk-dev] oej: trunk r128379 - /trunk/channels/chan_sip.c

[Russell Bryant]

On Jul 6, 2008, at 4:32 AM, SVN commits to the Digium repositories  
wrote:

> Author: oej
> Date: Sun Jul  6 03:32:11 2008
> New Revision: 128379
>
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=128379
> Log:
> Fix severe problem with my previous commit of "kill-the-user".  
> Russell saw a problem with this
> code, but not the correct problem. Thanks, anyway! ;-)
>
>
> Modified:
>    trunk/channels/chan_sip.c
>
> Modified: trunk/channels/chan_sip.c
> URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_sip.c?view=diff&rev=128379&r1=128378&r2=128379
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- trunk/channels/chan_sip.c (original)
> +++ trunk/channels/chan_sip.c Sun Jul  6 03:32:11 2008
> @@ -11672,8 +11672,8 @@
> 	/* First find device on name */
> 	peer = find_peer(of, NULL, TRUE, FALSE);
>
> -	/* Then find device on IP (if it's not a SUBSCRIBE) */
> -	if (sipmethod != SIP_SUBSCRIBE)
> +	/* If not found, then find device on IP (if it's not a SUBSCRIBE) */
> +	if (!peer && sipmethod != SIP_SUBSCRIBE)
> 		find_peer(NULL, &p->recv, TRUE, FALSE);


You haven't quite fixed this.  If find_peer() returns a result, then  
it is still being ignored.  If it gets ignored, you will have leaked a  
reference to the peer.  If you leak a reference, the peer will never  
be destroyed, and you have a memory leak.

You have two options.  One is to do what Tilghman suggested, which is  
to catch the result and immediately release the reference.  However,  
my question still remains; what is the point of calling the function  
if you don't care about the result?

I suspect you meant to have peer = find_peer() inside of that if  
statement.  :)

--
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.