[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] russell: branch 1.4 r114608 -
From:       tony () softins ! clara ! co ! uk (Tony Mountifield)
Date:       2008-04-25 9:01:39
Message-ID: fus6lj$asq$1 () softins ! clara ! co ! uk
[Download RAW message or body]

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

In article <20080424155521.EC03EA9DA24@lists.digium.internal>,
SVN commits to the Digium repositories <svn-commits@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@softins.co.uk - http://www.softins.co.uk
Play: tony@mountifield.org - http://tony.mountifield.org

_______________________________________________
--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


[prev in list] [next in list] [prev in thread] [next in thread]