chan_sip.c: Fixed off-nominal message iterator ref count and alloc fail issues.

* Fixed early exit in sip_msg_send() not destroying the message iterator.

* Made ast_msg_var_iterator_next() and ast_msg_var_iterator_destroy()
tolerant of a NULL iter parameter in case ast_msg_var_iterator_init()
fails.

* Made ast_msg_var_iterator_destroy() clean up any current message data
ref.

* Made struct ast_msg_var_iterator, ast_msg_var_iterator_init(),
ast_msg_var_iterator_next(), ast_msg_var_unref_current(), and
ast_msg_var_iterator_destroy() use iter instead of i.

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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index bbf7246fba0302fc7e57557b6be64c1067bf22d7..2125f068ca8babf90a3197e510873993f2e9b281 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -26995,12 +26995,11 @@ static int sip_msg_send(const struct ast_msg *msg, const char *to, const char *f
                return -1;
        }
 
-        for (iter = ast_msg_var_iterator_init(msg);
-            ast_msg_var_iterator_next(msg, iter, &var, &val);
-            ast_msg_var_unref_current(iter)) {
+       for (iter = ast_msg_var_iterator_init(msg);
+               ast_msg_var_iterator_next(msg, iter, &var, &val);
+               ast_msg_var_unref_current(iter)) {
                if (!strcasecmp(var, "Request-URI")) {
                        ast_string_field_set(pvt, fullcontact, val);
-                       ast_msg_var_unref_current(iter);
                        break;
                }
        }
@@ -27085,6 +27084,7 @@ static int sip_msg_send(const struct ast_msg *msg, const char *to, const char *f
                if (!strcasecmp(var, "Max-Forwards")) {
                        /* Decrement Max-Forwards for SIP loop prevention. */
                        if (sscanf(val, "%30d", &pvt->maxforwards) != 1 || pvt->maxforwards < 1) {
+                               ast_msg_var_iterator_destroy(iter);
                                sip_pvt_unlock(pvt);
                                dialog_unlink_all(pvt);
                                dialog_unref(pvt, "MESSAGE(Max-Forwards) reached zero.");

diff --git a/include/asterisk/message.h b/include/asterisk/message.h
index 31ed0b28a4e46291521b32736f2c655a190807c7..7e5c77a6a26a18c4221d2927e5ec07d760e9646d 100644 (file)
--- a/include/asterisk/message.h
+++ b/include/asterisk/message.h
@@ -167,6 +167,7 @@ int __attribute__((format(printf, 2, 3)))
  * \brief Set a variable on the message going to the dialplan.
  * \note Setting a variable that already exists overwrites the existing variable value
  *
+ * \param msg
  * \param name Name of variable to set
  * \param value Value of variable to set
  *
@@ -179,6 +180,7 @@ int ast_msg_set_var(struct ast_msg *msg, const char *name, const char *value);
  * \brief Set a variable on the message being sent to a message tech directly.
  * \note Setting a variable that already exists overwrites the existing variable value
  *
+ * \param msg
  * \param name Name of variable to set
  * \param value Value of variable to set
  *
@@ -244,25 +246,25 @@ struct ast_msg_var_iterator *ast_msg_var_iterator_init(const struct ast_msg *msg
 /*!
  * \brief Get the next variable name and value that is set for sending outbound
  * \param msg The message with the variables
- * \param i An iterator created with ast_msg_var_iterator_init
+ * \param iter An iterator created with ast_msg_var_iterator_init
  * \param name A pointer to the name result pointer
  * \param value A pointer to the value result pointer
  *
  * \retval 0 No more entries
  * \retval 1 Valid entry
  */
-int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *i, const char **name, const char **value);
+int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value);
 
 /*!
  * \brief Destroy a message variable iterator
- * \param i Iterator to be destroyed
+ * \param iter Iterator to be destroyed
  */
-void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *i);
+void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *iter);
 
 /*!
  * \brief Unref a message var from inside an iterator loop
  */
-void ast_msg_var_unref_current(struct ast_msg_var_iterator *i);
+void ast_msg_var_unref_current(struct ast_msg_var_iterator *iter);
 
 #if defined(__cplusplus) || defined(c_plusplus)
 }

diff --git a/main/message.c b/main/message.c
index 327d3b0c55ae7699bf92fd3ca8ab04b33f422f4d..0dcf031970216aca0440c94976bac6c7c42898e0 100644 (file)
--- a/main/message.c
+++ b/main/message.c
@@ -606,28 +606,34 @@ const char *ast_msg_get_var(struct ast_msg *msg, const char *name)
 }
 
 struct ast_msg_var_iterator {
-       struct ao2_iterator i;
+       struct ao2_iterator iter;
        struct msg_data *current_used;
 };
 
 struct ast_msg_var_iterator *ast_msg_var_iterator_init(const struct ast_msg *msg)
 {
-       struct ast_msg_var_iterator *i;
-       if (!(i = ast_calloc(1, sizeof(*i)))) {
+       struct ast_msg_var_iterator *iter;
+
+       iter = ast_calloc(1, sizeof(*iter));
+       if (!iter) {
                return NULL;
        }
 
-       i->i = ao2_iterator_init(msg->vars, 0);
+       iter->iter = ao2_iterator_init(msg->vars, 0);
 
-       return i;
+       return iter;
 }
 
-int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *i, const char **name, const char **value)
+int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value)
 {
        struct msg_data *data;
 
+       if (!iter) {
+               return 0;
+       }
+
        /* Skip any that aren't marked for sending out */
-       while ((data = ao2_iterator_next(&i->i)) && !data->send) {
+       while ((data = ao2_iterator_next(&iter->iter)) && !data->send) {
                ao2_ref(data, -1);
        }
 
@@ -642,22 +648,24 @@ int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iter
 
        /* Leave the refcount to be cleaned up by the caller with
         * ast_msg_var_unref_current after they finish with the pointers to the data */
-       i->current_used = data;
+       iter->current_used = data;
 
        return 1;
 }
 
-void ast_msg_var_unref_current(struct ast_msg_var_iterator *i) {
-       if (i->current_used) {
-               ao2_ref(i->current_used, -1);
-       }
-       i->current_used = NULL;
+void ast_msg_var_unref_current(struct ast_msg_var_iterator *iter)
+{
+       ao2_cleanup(iter->current_used);
+       iter->current_used = NULL;
 }
 
-void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *i)
+void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *iter)
 {
-       ao2_iterator_destroy(&i->i);
-       ast_free(i);
+       if (iter) {
+               ao2_iterator_destroy(&iter->iter);
+               ast_msg_var_unref_current(iter);
+               ast_free(iter);
+       }
 }
 
 static struct ast_channel *create_msg_q_chan(void)
@@ -1321,10 +1329,14 @@ void ast_msg_shutdown(void)
        }
 }
 
-/*! \internal \brief Clean up other resources on Asterisk shutdown
+/*!
+ * \internal
+ * \brief Clean up other resources on Asterisk shutdown
+ *
  * \note This does not include the msg_q_tp object, which must be disposed
  * of prior to Asterisk checking for channel destruction in its shutdown
- * sequence.  The atexit handlers are executed after this occurs. */
+ * sequence.  The atexit handlers are executed after this occurs.
+ */
 static void message_shutdown(void)
 {
        ast_custom_function_unregister(&msg_function);