[asterisk-dev] russell: branch 1.4 r114608 - /branches/1.4/channels/chan_iax2.c

[Tony Mountifield]

Just a bit of logical pedantry :-) see below...

In article <20080424155521.EC03EA9DA24 at lists.digium.internal>,
SVN commits to the Digium repositories <svn-commits at lists.digium.com> wrote:
> Author: russell
> Date: Thu Apr 24 10:55:21 2008
> New Revision: 114608
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=114608
> Log:
> Fix a silly mistake in a change I made yesterday that caused chan_iax2 to blow
> up very quickly.
> (issue #12515)
> 
> Modified:
>     branches/1.4/channels/chan_iax2.c
> 
> Modified: branches/1.4/channels/chan_iax2.c
> URL:
> http://svn.digium.com/view/asterisk/branches/1.4/channels/chan_iax2.c?view=diff&rev=114608&r1=114607&r2=114608
> ==============================================================================
> --- branches/1.4/channels/chan_iax2.c (original)
> +++ branches/1.4/channels/chan_iax2.c Thu Apr 24 10:55:21 2008
> @@ -1353,7 +1353,7 @@
>  					res = x;
>  				}
>  			}
> -			if (res && !return_locked)
> +			if (!res || (res && !return_locked))
>  				ast_mutex_unlock(&iaxsl[x]);

It will only reach the RHS of the || if res is true, so therefore "res &&"
is redundant, and the condition can be "if (!res || !return_locked)"

Looking at the surrounding code, I can see that two different ways are
being used to test for what is in fact the same condition.

In the unlock condition, a non-matched callno is tested for by !res, but
in the two for loops and the subsequent if, a non-matched callno is tested
for by (res < 1). I would favour changing (res < 1) to !res in those
instances, for consistency.

Alternatively, change the unlock condition to "if ((res < 1) || !return_locked)",
but I think the former is better.

Cheers
Tony

-- 
Tony Mountifield
Work: tony at softins.co.uk - http://www.softins.co.uk
Play: tony at mountifield.org - http://tony.mountifield.org