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

Joshua Colp reviewboard at asterisk.org
Tue May 14 18:12:36 CDT 2013



> On May 14, 2013, 10:56 p.m., Mark Michelson wrote:
> > /team/mmichelson/more_transfer/channels/chan_mgcp.c, lines 3272-3278
> > <https://reviewboard.asterisk.org/r/2536/diff/1/?file=37843#file37843line3272>
> >
> >     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?

It *should* be okay.


> On May 14, 2013, 10:56 p.m., Mark Michelson wrote:
> > /team/mmichelson/more_transfer/channels/chan_mgcp.c, line 3216
> > <https://reviewboard.asterisk.org/r/2536/diff/1/?file=37843#file37843line3216>
> >
> >     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.

I've added a bit more documentation but to truly understand this would require writing a lot about sub-channels, what they are, what they mean, and how they work. That is really what makes this complicated.


- Joshua


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


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/306a9948/attachment-0001.htm>


More information about the asterisk-dev mailing list