[asterisk-dev] [Code Review] SRTP support for Asterisk

[Mark Michelson]

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



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/191/#comment1257>

    I think you have a type mismatch here which may result in res having garbage data in it.
    
    Can't you just use
    pthread_join(thread1, ret);
    without any cast?



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/191/#comment1258>

    For CDR accuracy, you should change this ast_answer call to __ast_answer(chan, 0, 0);
    The second '0' argument will make sure that the CDR for the caller is not marked answered until the bridge begins.



/trunk/apps/app_dial.c
<http://reviewboard.digium.com/r/191/#comment1259>

    Same for this ast_answer as well.



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/191/#comment1261>

    Note that sip_srtp_destroy does not actually free the pointer passed in. You have a memory leak here as a result.



/trunk/channels/chan_sip.c
<http://reviewboard.digium.com/r/191/#comment1260>

    This function isn't really necessary since you can just use get_transport(). If the case of the returned string is important, I'd suggest modifying get_transport() to take another parameter stating which case is required.



/trunk/channels/sdp_crypto.c
<http://reviewboard.digium.com/r/191/#comment1262>

    Please enclose all the constants in parentheses.



/trunk/channels/sdp_crypto.c
<http://reviewboard.digium.com/r/191/#comment1263>

    Any idea what this is about?


- Mark


On 2009-03-10 14:39:03, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/191/
> -----------------------------------------------------------
> 
> (Updated 2009-03-10 14:39:03)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> SRTP support for Asterisk using Sdescriptions. This has been sitting around for a while, so I figured that it should at least get some review.  Full description of setup at http://lists.digium.com/pipermail/asterisk-dev/2009-January/036029.html
> 
> 
> This addresses bug 5413.
>     http://bugs.digium.com/view.php?id=5413
> 
> 
> Diffs
> -----
> 
>   /trunk/CREDITS 180812 
>   /trunk/apps/app_dial.c 180812 
>   /trunk/build_tools/menuselect-deps.in 180812 
>   /trunk/channels/Makefile 180812 
>   /trunk/channels/chan_iax2.c 180812 
>   /trunk/channels/chan_sip.c 180812 
>   /trunk/channels/sdp_crypto.h PRE-CREATION 
>   /trunk/channels/sdp_crypto.c PRE-CREATION 
>   /trunk/channels/sip_srtp.h PRE-CREATION 
>   /trunk/channels/sip_srtp.c PRE-CREATION 
>   /trunk/configure.ac 180812 
>   /trunk/funcs/func_channel.c 180812 
>   /trunk/include/asterisk/aes_internal.h 180812 
>   /trunk/include/asterisk/autoconfig.h.in 180812 
>   /trunk/include/asterisk/rtp.h 180812 
>   /trunk/main/rtp.c 180812 
>   /trunk/makeopts.in 180812 
>   /trunk/res/res_srtp.c PRE-CREATION 
> 
> Diff: http://reviewboard.digium.com/r/191/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Terry
> 
>