[asterisk-dev] [Code Review] Avoid valgrind warnings for ast_rtp_instance_get_xxx_address

[rmudgett at digium.com]

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

Ship it!


This turned out to be a much smaller change than I originally thought.

- rmudgett


On 2010-11-03 11:44:11, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/995/
> -----------------------------------------------------------
> 
> (Updated 2010-11-03 11:44:11)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Summary
> -------
> 
> The documentation for ast_rtp_instance_get_(local/remote)_address stated that they returned 0 for success and -1 on failure. Instead, they returned 0 if the address structure passed in was already equivalent to the address instance local/remote address or 1 otherwise. 90% of the calls to these functions completely ignored the return address and passed in an uninitialized struct, which would make valgrind complain even though the operation was technically safe.
> 
> This patch fixes the documentation and converts the get_xxx_address functions to void since all they really do is copy the address and cannot fail. Additionally two new functions (ast_rtp_instance_get_and_cmp_(local/remote)_address) are created for the 3 times where the return value was actually checked. The get_and_cmp_local_address function is currently unused, but exists for the sake of symmetry.
> 
> The only functional change as a result of this change is that we will not do an ast_sockaddr_cmp() on (mostly uninitialized) addresses before doing the ast_sockaddr_copy() in the get_*_address functions. So, even though it is an API change, it shouldn't have a noticeable change in behavior.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 293562 
>   /branches/1.8/include/asterisk/rtp_engine.h 293562 
>   /branches/1.8/main/rtp_engine.c 293562 
> 
> Diff: https://reviewboard.asterisk.org/r/995/diff
> 
> 
> Testing
> -------
> 
> Asterisk compiles and runs.
> 
> 
> Thanks,
> 
> Terry
> 
>