[asterisk-commits] dvossel: branch 10 r346349 - in /branches/10: include/asterisk/ main/

[SVN commits to the Asterisk project]

Author: dvossel
Date: Mon Nov 28 18:00:11 2011
New Revision: 346349

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=346349
Log:
Fixes memory leak in message API.

The ast_msg_get_var function did not properly decrement
the ref count of the var it retrieves.  The way this is
implemented is a bit tricky, as we must decrement the var and then
return the var's value.  As long as the documentation for the
function is followed, this will not result in a dangling pointer as
the ast_msg structure owns its own reference to the var while it
exists in the var container.

Modified:
    branches/10/include/asterisk/message.h
    branches/10/main/message.c

Modified: branches/10/include/asterisk/message.h
URL: http://svnview.digium.com/svn/asterisk/branches/10/include/asterisk/message.h?view=diff&rev=346349&r1=346348&r2=346349
==============================================================================
--- branches/10/include/asterisk/message.h (original)
+++ branches/10/include/asterisk/message.h Mon Nov 28 18:00:11 2011
@@ -173,7 +173,8 @@
 /*!
  * \brief Get the specified variable on the message
  * \note The return value is valid only as long as the ast_message is valid. Hold a reference
- *       to the message if you plan on storing the return value. 
+ *       to the message if you plan on storing the return value. Do re-set the same
+ *       message var name while holding a pointer to the result of this function.
  *
  * \return The value associated with variable "name". NULL if variable not found.
  */

Modified: branches/10/main/message.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/main/message.c?view=diff&rev=346349&r1=346348&r2=346349
==============================================================================
--- branches/10/main/message.c (original)
+++ branches/10/main/message.c Mon Nov 28 18:00:11 2011
@@ -522,12 +522,20 @@
 const char *ast_msg_get_var(struct ast_msg *msg, const char *name)
 {
 	struct msg_data *data;
+	const char *val = NULL;
 
 	if (!(data = msg_data_find(msg->vars, name))) {
 		return NULL;
 	}
 
-	return data->value;
+	/* Yep, this definitely looks like val would be a dangling pointer
+	 * after the ref count is decremented.  As long as the message structure
+	 * is used in a thread safe manner, this will not be the case though.
+	 * The ast_msg holds a reference to this object in the msg->vars container. */
+	val = data->value;
+	ao2_ref(data, -1);
+
+	return val;
 }
 
 struct ast_msg_var_iterator {