[asterisk-dev] [Code Review] Merge Called party identification changes into trunk.

[Russell Bryant]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/201/#review597
-----------------------------------------------------------



/trunk/apps/app_queue.c
<http://reviewboard.digium.com/r/201/#comment1486>

    This could be declared as a boolean.



/trunk/apps/app_queue.c
<http://reviewboard.digium.com/r/201/#comment1487>

    This section needs some channel locking love.



/trunk/apps/app_queue.c
<http://reviewboard.digium.com/r/201/#comment1488>

    Channel locking needed here



/trunk/apps/app_queue.c
<http://reviewboard.digium.com/r/201/#comment1489>

    Channel locking needed here



/trunk/apps/app_queue.c
<http://reviewboard.digium.com/r/201/#comment1490>

    Channel locking



/trunk/channels/chan_misdn.c
<http://reviewboard.digium.com/r/201/#comment1493>

    There are still a lot of these "end" comments that need to be stripped before we can merge this.



/trunk/channels/chan_misdn.c
<http://reviewboard.digium.com/r/201/#comment1494>

    It would be nice to stick these values in an enum.



/trunk/channels/chan_misdn.c
<http://reviewboard.digium.com/r/201/#comment1495>

    There are some whitespace issues here, and in some other places in this file.



/trunk/channels/chan_misdn.c
<http://reviewboard.digium.com/r/201/#comment1496>

    This is pretty pedantic, but instead of 0, I prefer '\0'.



/trunk/channels/chan_misdn.c
<http://reviewboard.digium.com/r/201/#comment1497>

    The ast_str API makes doing this type of string manipulation much easier.  This would just become a call to ast_str_append().



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/201/#comment1498>

    Be sure to get all of this pesky whitespace stuff before the final merge.



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/201/#comment1500>

    I'm not sure if I commented on this already, but this function (add_rpid()) assumes that p->owner is locked.  Could you please verify that this is handled properly?  Also, after doing so, document p->owner being locked as a precondition.



/trunk/funcs/func_redirecting.c
<http://reviewboard.digium.com/r/201/#comment1485>

    There are some errors in this XML documentation.  Run "make validate-docs" to see them.


- Russell


On 2009-03-23 09:52:15, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/201/
> -----------------------------------------------------------
> 
> (Updated 2009-03-23 09:52:15)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This diff encompasses the changes between the issue8824 branch and Asterisk trunk. These changes include all the changes necessary to allow for the transmission and reception of remote called party identification (COLP/CONP).
> 
> 
> This addresses bug 8824.
>     http://bugs.digium.com/view.php?id=8824
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_dial.c 183691 
>   /trunk/apps/app_directed_pickup.c 183691 
>   /trunk/apps/app_queue.c 183691 
>   /trunk/channels/chan_agent.c 183691 
>   /trunk/channels/chan_dahdi.c 183691 
>   /trunk/channels/chan_h323.c 183691 
>   /trunk/channels/chan_iax2.c 183691 
>   /trunk/channels/chan_local.c 183691 
>   /trunk/channels/chan_mgcp.c 183691 
>   /trunk/channels/chan_misdn.c 183691 
>   /trunk/channels/chan_phone.c 183691 
>   /trunk/channels/chan_sip.c 183691 
>   /trunk/channels/chan_skinny.c 183691 
>   /trunk/channels/chan_unistim.c 183691 
>   /trunk/channels/misdn/chan_misdn_config.h 183691 
>   /trunk/channels/misdn/isdn_lib.h 183691 
>   /trunk/channels/misdn/isdn_lib.c 183691 
>   /trunk/channels/misdn/isdn_lib_intern.h 183691 
>   /trunk/channels/misdn/isdn_msg_parser.c 183691 
>   /trunk/channels/misdn_config.c 183691 
>   /trunk/configs/misdn.conf.sample 183691 
>   /trunk/configs/sip.conf.sample 183691 
>   /trunk/funcs/func_connectedline.c PRE-CREATION 
>   /trunk/funcs/func_redirecting.c PRE-CREATION 
>   /trunk/include/asterisk/callerid.h 183691 
>   /trunk/include/asterisk/channel.h 183691 
>   /trunk/include/asterisk/frame.h 183691 
>   /trunk/main/callerid.c 183691 
>   /trunk/main/channel.c 183691 
>   /trunk/main/dial.c 183691 
>   /trunk/main/features.c 183691 
>   /trunk/main/rtp.c 183691 
> 
> Diff: http://reviewboard.digium.com/r/201/diff
> 
> 
> Testing
> -------
> 
> Digium's Product Quality department has extensively tested a remarkably similar version of these enhancements. In addition, we know of a customer who has been using this branch for some months now in a production environment. I would also be willing to bet that some of those who have been monitoring issue 8824 have also done some testing as well.
> 
> 
> Thanks,
> 
> Mark
> 
>