[svn-commits] rmudgett: branch group/media_formats-reviewed-trunk r418418 - in /team/group/...

[SVN commits to the Digium repositories]

Author: rmudgett
Date: Fri Jul 11 17:46:10 2014
New Revision: 418418

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=418418
Log:
translate.c: Fix ref leaks in ast_translator_best_choice().

* Fixed ref leaks of bestdst and joint_cap in
ast_translator_best_choice().

* Added BUGBUG notes in format.c about objects allocated with locks that
don't need them.

* Fixed a couple doxygen comments in format.h.

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

Modified:
    team/group/media_formats-reviewed-trunk/include/asterisk/format.h
    team/group/media_formats-reviewed-trunk/main/format.c
    team/group/media_formats-reviewed-trunk/main/translate.c

Modified: team/group/media_formats-reviewed-trunk/include/asterisk/format.h
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/include/asterisk/format.h?view=diff&rev=418418&r1=418417&r2=418418
==============================================================================
--- team/group/media_formats-reviewed-trunk/include/asterisk/format.h (original)
+++ team/group/media_formats-reviewed-trunk/include/asterisk/format.h Fri Jul 11 17:46:10 2014
@@ -48,7 +48,7 @@
 	 */
 	void (*const format_destroy)(struct ast_format *format);
 
-	/**
+	/*!
 	 * \brief Callback for when the format is cloned, used to clone attributes
 	 *
 	 * \param src Source format of attributes
@@ -97,7 +97,7 @@
 	struct ast_format *(* const format_attribute_set)(const struct ast_format *format,
 		const char *name, const char *value);
 
-	/*
+	/*!
 	 * \brief Parse SDP attribute information, interpret it, and store it in the format structure.
 	 *
 	 * \param format Format to set attributes on

Modified: team/group/media_formats-reviewed-trunk/main/format.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/format.c?view=diff&rev=418418&r1=418417&r2=418418
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/format.c (original)
+++ team/group/media_formats-reviewed-trunk/main/format.c Fri Jul 11 17:46:10 2014
@@ -148,7 +148,7 @@
 		return -1;
 	}
 
-	format_interface = ao2_alloc(sizeof(*format_interface) + strlen(codec) + 1, NULL);
+	format_interface = ao2_alloc(sizeof(*format_interface) + strlen(codec) + 1, NULL);/* BUGBUG this doesn't need a lock. */
 	if (!format_interface) {
 		return -1;
 	}
@@ -190,7 +190,7 @@
 	struct ast_format *format;
 	struct format_interface *format_interface;
 
-	format = ao2_alloc(sizeof(*format), format_destroy);
+	format = ao2_alloc(sizeof(*format), format_destroy);/* BUGBUG this doesn't need a lock. */
 	if (!format) {
 		return NULL;
 	}

Modified: team/group/media_formats-reviewed-trunk/main/translate.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/translate.c?view=diff&rev=418418&r1=418417&r2=418418
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/translate.c (original)
+++ team/group/media_formats-reviewed-trunk/main/translate.c Fri Jul 11 17:46:10 2014
@@ -1262,18 +1262,13 @@
 	struct ast_format **dst_fmt_out,
 	struct ast_format **src_fmt_out)
 {
-	/*
-	 * BUGBUG:
-	 *   This procedure was directly copied from main/translate.c in the
-	 *   media_formats-reviewed (based on /branches/12).  It compiles but
-	 *   should be double-checked.
-	 */
 	unsigned int besttablecost = INT_MAX;
 	unsigned int beststeps = INT_MAX;
-	struct ast_format *best = NULL;
-	struct ast_format *bestdst = NULL;
-	struct ast_format_cap *joint_cap;
-	int i, j;
+	RAII_VAR(struct ast_format *, best, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_format *, bestdst, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_format_cap *, joint_cap, NULL, ao2_cleanup);
+	int i;
+	int j;
 
 	if (!(joint_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT))) {
 		return -1;
@@ -1304,8 +1299,6 @@
 	if (best) {
 		ao2_replace(*dst_fmt_out, best);
 		ao2_replace(*src_fmt_out, best);
-		ao2_ref(best, -1);
-		ao2_ref(joint_cap, -1);
 		return 0;
 	}
 	/* need to translate */
@@ -1354,16 +1347,8 @@
 	if (!best) {
 		return -1;
 	}
-	/* BUGBUG:
-	 *    What if *dst_fmt_out / *src_fmt_out are already holding refs
-	 *    We need to audit callers, I beleive I've seen leaks from 
-	 *    ast_translator_best_choice but ao2_replace causes undefined variable
-	 *    access (reported by valgrind).
-	 */
 	ao2_replace(*dst_fmt_out, bestdst);
 	ao2_replace(*src_fmt_out, best);
-	ao2_ref(best, -1);
-	ao2_ref(bestdst, -1);
 	return 0;
 }