[asterisk-dev] [Code Review] 2536: Moving chan_mgcp to attended transfer API

[Mark Michelson]

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



/team/mmichelson/more_transfer/channels/chan_mgcp.c
<https://reviewboard.asterisk.org/r/2536/#comment16796>

    Since you have taken the time to unravel the mysteries behind MGCP's transfers, please please please add some doxygen to this particular function. Focus on
    
    1) What the different parties are (p->sub, p->sub->next, sub, and sub->next)
    2) What the different return values mean.
    
    As it is, this is incredibly difficult to understand, especially the return values. Looking at how the return of attempt_transfer() is handled doesn't really help matters any.



/team/mmichelson/more_transfer/channels/chan_mgcp.c
<https://reviewboard.asterisk.org/r/2536/#comment16797>

    Since sub->next is unalloced no matter what, could you do this:
    
    unalloc_sub(sub->next);
    
    if (p->sub != sub) {
        p->sub = sub;
        return 1;
    }
    
    return 0;
    
    
    Or would that ordering mess things up?


- Mark Michelson


On May 14, 2013, 3:21 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2536/
> -----------------------------------------------------------
> 
> (Updated May 14, 2013, 3:21 p.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change cleans up the MGCP transfer code a bit, moves it to the new attended transfer API, and adds a few comments.
> 
> 
> Diffs
> -----
> 
>   /team/mmichelson/more_transfer/channels/chan_mgcp.c 388687 
> 
> Diff: https://reviewboard.asterisk.org/r/2536/diff/
> 
> 
> Testing
> -------
> 
> I wish.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130514/93f9236d/attachment.htm>