[Asterisk-code-review] vector: multiple evaluation of elem in AST VECTOR ADD SORTED. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/6661 )

Change subject: vector: multiple evaluation of elem in AST_VECTOR_ADD_SORTED.
......................................................................

vector: multiple evaluation of elem in AST_VECTOR_ADD_SORTED.

Use temporary variable to prevent multiple evaluations of elem argument.
This resolves a memory leak in res_pjproject startup.

ASTERISK-27317 #close

Change-Id: Ib960d7f5576f9e1a3c478ecb48995582a574e06d
---
M include/asterisk/vector.h
M tests/test_vector.c
2 files changed, 8 insertions(+), 4 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Joshua Colp: Approved for Submit



diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h
index 2de84d2..1e6fe03 100644
--- a/include/asterisk/vector.h
+++ b/include/asterisk/vector.h
@@ -304,27 +304,31 @@
  * \brief Add an element into a sorted vector
  *
  * \param vec Sorted vector to add to.
- * \param elem Element to insert.
+ * \param elem Element to insert. Must not be an array type.
  * \param cmp A strcmp compatible compare function.
  *
  * \return 0 on success.
  * \return Non-zero on failure.
  *
  * \warning Use of this macro on an unsorted vector will produce unpredictable results
+ * \warning 'elem' must not be an array type so passing 'x' where 'x' is defined as
+ *          'char x[4]' will fail to compile. However casting 'x' as 'char *' does
+ *          result in a value that CAN be used.
  */
 #define AST_VECTOR_ADD_SORTED(vec, elem, cmp) ({ \
 	int res = 0; \
 	size_t __idx = (vec)->current; \
+	typeof(elem) __elem = (elem); \
 	do { \
 		if (__make_room((vec)->current, vec) != 0) { \
 			res = -1; \
 			break; \
 		} \
-		while (__idx > 0 && (cmp((vec)->elems[__idx - 1], elem) > 0)) { \
+		while (__idx > 0 && (cmp((vec)->elems[__idx - 1], __elem) > 0)) { \
 			(vec)->elems[__idx] = (vec)->elems[__idx - 1]; \
 			__idx--; \
 		} \
-		(vec)->elems[__idx] = elem; \
+		(vec)->elems[__idx] = __elem; \
 		(vec)->current++; \
 	} while (0); \
 	res; \
diff --git a/tests/test_vector.c b/tests/test_vector.c
index 8e0d121..2dfcc60 100644
--- a/tests/test_vector.c
+++ b/tests/test_vector.c
@@ -210,7 +210,7 @@
 	ast_test_validate_cleanup(test, AST_VECTOR_ADD_SORTED(&sv1, ZZZ, strcmp) == 0, rc, cleanup);
 	ast_test_validate_cleanup(test, AST_VECTOR_ADD_SORTED(&sv1, CCC, strcmp) == 0, rc, cleanup);
 	ast_test_validate_cleanup(test, AST_VECTOR_ADD_SORTED(&sv1, AAA, strcmp) == 0, rc, cleanup);
-	ast_test_validate_cleanup(test, AST_VECTOR_ADD_SORTED(&sv1, CCC2, strcmp) == 0, rc, cleanup);
+	ast_test_validate_cleanup(test, AST_VECTOR_ADD_SORTED(&sv1, (char*)CCC2, strcmp) == 0, rc, cleanup);
 
 	ast_test_validate_cleanup(test, AST_VECTOR_GET(&sv1, 0) == AAA, rc, cleanup);
 	ast_test_validate_cleanup(test, AST_VECTOR_GET(&sv1, 1) == BBB, rc, cleanup);

-- 
To view, visit https://gerrit.asterisk.org/6661
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib960d7f5576f9e1a3c478ecb48995582a574e06d
Gerrit-Change-Number: 6661
Gerrit-PatchSet: 3
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171007/8bf3994c/attachment.html>