[asterisk-dev] [Code Review] Add Calling and Called subaddress support for Asterisk apps and funcs

[rmudgett at digium.com]

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



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2744>

    I thought there was a similar function to allocate from the stack.  The function turns out to be alloca().  Unfortunately, that function may not be available on all platforms.  However, what you did turns out better anyway.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2745>

    Change to:
    ast_subaddress->str = cnum
    Fixes the memory leak and avoids an allocation and copy as a bonus.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2746>

    Only one \brief is needed.  If you want to add more details you should add a \details paragraph to provide a more verbose description of what the function does.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2749>

    A maximum buffer size for dst is needed to avoid overrun.  A real possibility since the source string can be set extremely long by the dialplan.
    
    res can be calculated rather than incremented:
    len = strlen(src);
    res = len / 2 + len % 2;



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2747>

    dst and src do not need to be incremented since they are no longer needed.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2748>

    These if test can be combined to reduce indentation.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2750>

    These two tests can be combined to reduce indentation.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2751>

    Do you still need this channel variable?
    
    If so, you can reduce the indentation by combining the tests as above.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2752>

    These two tests can be combined to reduce indentation.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2753>

    Do you still need this channel variable?
    
    If so, you can reduce the indentation by combining the tests as above.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2754>

    To reduce the possibility of finding the wrong ':' you should first search for the '/'.
    
    dest is ---v
    Dial(DAHDI/pseudo[/extension])
    Dial(DAHDI/<channel#>[c|r<cadance#>|d][/extension])
    Dial(DAHDI/(g|G|r|R)<group#(0-63)>[c|r<cadance#>|d][/extension])
    



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2756>

    To get the calling subaddres into libpri we need to use the pri_sr_set_caller_party() here.
    
    #if defined(HAVE_PRI_SUBADDR)
    fill in a struct pri_party_id here.
    pri_sr_set_caller_party()
    #else
    <original code>
    #endif /* defined(HAVE_PRI_SUBADDR) */
    
    sig_pri_party_id_from_ast() might be able to do most of the work.



/team/rmudgett/subaddr/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/405/#comment2755>

    Move this to where pri_sr_set_called() is called above so we deal with the called party in the same place.


- rmudgett


On 2009-10-16 06:55:54, alecdavis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/405/
> -----------------------------------------------------------
> 
> (Updated 2009-10-16 06:55:54)
> 
> 
> Review request for Asterisk Developers, rmudgett and mattf.
> 
> 
> Summary
> -------
> 
> Add dialstring support for called subaddress, used like Dial(dahdi/g0/5551234:8512) where 5551234 is the main line number of the remote site and 8512 is the extension with that site.
> If 'User Specific' BCD encoding is required, the subaddress is prefixed with a 'U'. IE Dial(dahdi/g0/5551234:U8512)
> 
> Extend function CALLERID() for type 'subaddr' to allow reading and updating the calling subaddress.
> Extend function CALLERID() for type 'dnid-subaddr' to allow reading and updating the called subaddress.
> Extend function CONNECTEDLINE as for CALLERID
> 
> 
> This addresses bug 15604.
>     https://issues.asterisk.org/view.php?id=15604
> 
> 
> Diffs
> -----
> 
>   /team/rmudgett/subaddr/channels/sig_pri.c 224259 
>   /team/rmudgett/subaddr/funcs/func_callerid.c 224259 
>   /team/rmudgett/subaddr/funcs/func_connectedline.c 224259 
>   /team/rmudgett/subaddr/include/asterisk/channel.h 224259 
>   /team/rmudgett/subaddr/main/channel.c 224259 
> 
> Diff: https://reviewboard.asterisk.org/r/405/diff
> 
> 
> Testing
> -------
> 
> Preliminary concept testing between sites over Telecom NZ PSTN, using channel variables $CALLINGSUBADDR and $CALLEDSUBADDR with patches for branches at https://issues.asterisk.org/view.php?id=15604 
> Development and testing using Jtec 5015 switches, establish calls in either direction, NSAP and User Specific.
> 
> 
> Thanks,
> 
> alecdavis
> 
>