[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