[asterisk-dev] [Code Review]: Don't use a variable after calling ASTOBJ_UNREF on it.

rmudgett reviewboard at asterisk.org
Mon May 21 19:28:24 CDT 2012



> On May 21, 2012, 6:37 p.m., rmudgett wrote:
> > /branches/1.8/channels/chan_sip.c, line 12428
> > <https://reviewboard.asterisk.org/r/1940/diff/1/?file=28179#file28179line12428>
> >
> >     Rather than cramming as much as you can into a single statement, why not keep the separate statements like it was before?
> >     
> >     Remember the KISS principle.
> 
> Terry Wilson wrote:
>     I initially had it the other way. To me this was clearer that we are saving a ref specifically for this function call. Although I still prefer this way, if you feel strongly about it I can change it.

I have had to fix a few places in Asterisk where the assignment was crammed into the if test and the parentheses were misplaced.
if ((a = b < 0))
when it should have been
if ((a = b) < 0)

Breaking the assignment and test into two statements avoids the problem altogether.  It also gives better places to break long lines.

Although C is not lisp, you can still get Lost In Stupid Parentheses. :)


- rmudgett


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


On May 21, 2012, 3:52 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1940/
> -----------------------------------------------------------
> 
> (Updated May 21, 2012, 3:52 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Summary
> -------
> 
> This just saves the ref to a variable and unrefs that instead.
> 
> 
> This addresses bug ASTERISK-19827.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19827
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 367176 
> 
> Diff: https://reviewboard.asterisk.org/r/1940/diff
> 
> 
> Testing
> -------
> 
> Compiles.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120522/89ca5b72/attachment-0001.htm>


More information about the asterisk-dev mailing list