Avoid valgrind warnings for ast_rtp_instance_get_xxx_address

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.

Review: https://reviewboard.asterisk.org/r/995/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@293803 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 0fa510e1ee0a7c08d8945258d6bf0c22092d480f..ff9a8617f8b4ac1b9c0ceb5a71bbf8d94112a918 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -27564,19 +27564,19 @@ static int sip_set_rtp_peer(struct ast_channel *chan, struct ast_rtp_instance *i
        }
 
        if (instance) {
-               changed |= ast_rtp_instance_get_remote_address(instance, &p->redirip);
+               changed |= ast_rtp_instance_get_and_cmp_remote_address(instance, &p->redirip);
        } else if (!ast_sockaddr_isnull(&p->redirip)) {
                memset(&p->redirip, 0, sizeof(p->redirip));
                changed = 1;
        }
        if (vinstance) {
-               changed |= ast_rtp_instance_get_remote_address(vinstance, &p->vredirip);
+               changed |= ast_rtp_instance_get_and_cmp_remote_address(vinstance, &p->vredirip);
        } else if (!ast_sockaddr_isnull(&p->vredirip)) {
                memset(&p->vredirip, 0, sizeof(p->vredirip));
                changed = 1;
        }
        if (tinstance) {
-               changed |= ast_rtp_instance_get_remote_address(tinstance, &p->tredirip);
+               changed |= ast_rtp_instance_get_and_cmp_remote_address(tinstance, &p->tredirip);
        } else if (!ast_sockaddr_isnull(&p->tredirip)) {
                memset(&p->tredirip, 0, sizeof(p->tredirip));
                changed = 1;

diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index 74ea34668c8bb7d7a511d712a163fbbb2186e634..39633c84c8ef4d8e72d123fb25e403613083dc06 100644 (file)
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -720,9 +720,6 @@ int ast_rtp_instance_set_local_address(struct ast_rtp_instance *instance,
  * \param instance The RTP instance to get the address from
  * \param address The variable to store the address in
  *
- * \retval 0 success
- * \retval -1 failure
- *
  * Example usage:
  *
  * \code
@@ -734,7 +731,30 @@ int ast_rtp_instance_set_local_address(struct ast_rtp_instance *instance,
  *
  * \since 1.8
  */
-int ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+void ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+
+/*!
+ * \brief Get the address of the local endpoint that we are sending RTP to, comparing its address to another
+ *
+ * \param instance The instance that we want to get the local address for
+ * \param address An initialized address that may be overwritten if the local address is different
+ *
+ * \retval 0 address was not changed
+ * \retval 1 address was changed
+ * Example usage:
+ *
+ * \code
+ * struct ast_sockaddr address;
+ * int ret;
+ * ret = ast_rtp_instance_get_and_cmp_local_address(instance, &address);
+ * \endcode
+ *
+ * This retrieves the current local address set on the instance pointed to by instance and puts the value
+ * into the address structure.
+ *
+ * \since 1.8
+ */
+int ast_rtp_instance_get_and_cmp_local_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
 
 /*!
  * \brief Get the address of the remote endpoint that we are sending RTP to
@@ -742,9 +762,6 @@ int ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance, struct
  * \param instance The instance that we want to get the remote address for
  * \param address A structure to put the address into
  *
- * \retval 0 success
- * \retval -1 failure
- *
  * Example usage:
  *
  * \code
@@ -757,7 +774,31 @@ int ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance, struct
  *
  * \since 1.8
  */
-int ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+void ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
+
+/*!
+ * \brief Get the address of the remote endpoint that we are sending RTP to, comparing its address to another
+ *
+ * \param instance The instance that we want to get the remote address for
+ * \param address An initialized address that may be overwritten if the remote address is different
+ *
+ * \retval 0 address was not changed
+ * \retval 1 address was changed
+ * Example usage:
+ *
+ * \code
+ * struct ast_sockaddr address;
+ * int ret;
+ * ret = ast_rtp_instance_get_and_cmp_remote_address(instance, &address);
+ * \endcode
+ *
+ * This retrieves the current remote address set on the instance pointed to by instance and puts the value
+ * into the address structure.
+ *
+ * \since 1.8
+ */
+
+int ast_rtp_instance_get_and_cmp_remote_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address);
 
 /*!
  * \brief Set the value of an RTP instance extended property

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index ff8ebd4813baff72263d4133481e225d2202ff09..0fecff8cf97d85b46b24705523f67a63c4e38064 100644 (file)
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -410,7 +410,7 @@ int ast_rtp_instance_set_alt_remote_address(struct ast_rtp_instance *instance,
        return 0;
 }
 
-int ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance,
+int ast_rtp_instance_get_and_cmp_local_address(struct ast_rtp_instance *instance,
                struct ast_sockaddr *address)
 {
        if (ast_sockaddr_cmp(address, &instance->local_address) != 0) {
@@ -421,7 +421,13 @@ int ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance,
        return 0;
 }
 
-int ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance,
+void ast_rtp_instance_get_local_address(struct ast_rtp_instance *instance,
+               struct ast_sockaddr *address)
+{
+       ast_sockaddr_copy(address, &instance->local_address);
+}
+
+int ast_rtp_instance_get_and_cmp_remote_address(struct ast_rtp_instance *instance,
                struct ast_sockaddr *address)
 {
        if (ast_sockaddr_cmp(address, &instance->remote_address) != 0) {
@@ -432,6 +438,12 @@ int ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance,
        return 0;
 }
 
+void ast_rtp_instance_get_remote_address(struct ast_rtp_instance *instance,
+               struct ast_sockaddr *address)
+{
+       ast_sockaddr_copy(address, &instance->remote_address);
+}
+
 void ast_rtp_instance_set_extended_prop(struct ast_rtp_instance *instance, int property, void *value)
 {
        if (instance->engine->extended_prop_set) {