[Asterisk-code-review] test_abstract_jb.c: Fix put and put_out_of_order memory leaks. (asterisk[19])

George Joseph asteriskteam at digium.com
Fri Sep 10 14:26:38 CDT 2021


George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/16452 )

Change subject: test_abstract_jb.c: Fix put and put_out_of_order memory leaks.
......................................................................

test_abstract_jb.c: Fix put and put_out_of_order memory leaks.

We can't rely on RAII_VAR(...) to properly clean up data that is
allocated within a loop.

ASTERISK-27176 #close

Change-Id: Ib575616101230c4f603519114ec62ebf3936882c
---
M tests/test_abstract_jb.c
1 file changed, 28 insertions(+), 9 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/tests/test_abstract_jb.c b/tests/test_abstract_jb.c
index 53614bc..76eb4bf 100644
--- a/tests/test_abstract_jb.c
+++ b/tests/test_abstract_jb.c
@@ -293,8 +293,8 @@
 	RAII_VAR(struct ast_jb *, jb, &default_jb, dispose_jitterbuffer); \
 	const struct ast_jb_impl *impl; \
 	struct ast_jb_conf conf; \
-	RAII_VAR(struct ast_frame *, expected_frame, NULL, ast_frame_dtor); \
-	RAII_VAR(struct ast_frame *, actual_frame, NULL, ast_frame_dtor); \
+	struct ast_frame *expected_frame = NULL; \
+	struct ast_frame *actual_frame = NULL; \
 	int res; \
 	long next; \
 	int i; \
@@ -318,7 +318,15 @@
 	jb->impl = impl; \
 \
 	expected_frame = create_test_frame(1000, 0); \
-	jb->impl->put_first(jb->jbobj, expected_frame, 1100); \
+	res = jb->impl->put_first(jb->jbobj, \
+		expected_frame, \
+		1100); \
+	if (res != AST_JB_IMPL_OK) { \
+		ast_test_status_update(test, "Error: On first frame, got %d back from put_first (expected %d)\n", \
+			res, AST_JB_IMPL_OK); \
+		ast_frame_dtor(expected_frame); \
+		return AST_TEST_FAIL; \
+	} \
 	for (i = 1; i < 10; i++) { \
 		expected_frame = create_test_frame(1000 + i * DEFAULT_FRAME_MS, 0); \
 		res = jb->impl->put(jb->jbobj, \
@@ -327,6 +335,7 @@
 		if (res != AST_JB_IMPL_OK) { \
 			ast_test_status_update(test, "Error: On frame %d, got %d back from put (expected %d)\n", \
 				i, res, AST_JB_IMPL_OK); \
+			ast_frame_dtor(expected_frame); \
 			return AST_TEST_FAIL; \
 		} \
 	} \
@@ -338,11 +347,12 @@
 		if (res != AST_JB_IMPL_OK) { \
 			ast_test_status_update(test, "Error: failed to retrieve frame %i at time %ld\n", \
 				i, next); \
+			ast_frame_dtor(expected_frame); \
 			return AST_TEST_FAIL; \
 		} \
 		VERIFY_FRAME(actual_frame, expected_frame); \
-		ast_frfree(expected_frame); \
-		expected_frame = NULL; \
+		ast_frame_dtor(expected_frame); \
+		ast_frame_dtor(actual_frame); \
 	} \
 	return AST_TEST_PASS; \
 }
@@ -427,8 +437,8 @@
 	RAII_VAR(struct ast_jb *, jb, &default_jb, dispose_jitterbuffer); \
 	const struct ast_jb_impl *impl; \
 	struct ast_jb_conf conf; \
-	RAII_VAR(struct ast_frame *, actual_frame, NULL, ast_frame_dtor); \
-	RAII_VAR(struct ast_frame *, expected_frame, NULL, ast_frame_dtor); \
+	struct ast_frame *expected_frame = NULL; \
+	struct ast_frame *actual_frame = NULL; \
 	int res; \
 	long next; \
 	int i; \
@@ -454,7 +464,13 @@
 	jb->impl = impl; \
 \
 	expected_frame = create_test_frame(1000, 0); \
-	jb->impl->put_first(jb->jbobj, expected_frame, 1100); \
+	res = jb->impl->put_first(jb->jbobj, expected_frame, 1100); \
+	if (res != AST_JB_IMPL_OK) { \
+		ast_test_status_update(test, "Error: On first frame, got %d back from put_first (expected %d)\n", \
+			res, AST_JB_IMPL_OK); \
+		ast_frame_dtor(expected_frame); \
+		return AST_TEST_FAIL; \
+	} \
 	for (i = 1; i <= 10; i++) { \
 		if (i % 3 == 1 && i != 10) { \
 			expected_frame = create_test_frame(1000 + ((i + 1) * DEFAULT_FRAME_MS), 0); \
@@ -469,6 +485,7 @@
 		if (res != AST_JB_IMPL_OK) { \
 			ast_test_status_update(test, "Error: On frame %d, got %d back from put (expected %d)\n", \
 				i, res, AST_JB_IMPL_OK); \
+			ast_frame_dtor(expected_frame); \
 			return AST_TEST_FAIL; \
 		} \
 	} \
@@ -480,10 +497,12 @@
 		if (res != AST_JB_IMPL_OK) { \
 			ast_test_status_update(test, "Error: failed to retrieve frame at %ld\n", \
 				next); \
+			ast_frame_dtor(expected_frame); \
 			return AST_TEST_FAIL; \
 		} \
 		VERIFY_FRAME(actual_frame, expected_frame); \
-		ast_frfree(expected_frame); \
+		ast_frame_dtor(expected_frame); \
+		ast_frame_dtor(actual_frame); \
 		expected_frame = NULL; \
 	} \
 \

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/16452
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 19
Gerrit-Change-Id: Ib575616101230c4f603519114ec62ebf3936882c
Gerrit-Change-Number: 16452
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Bright <sean at seanbright.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210910/37cb5b2b/attachment-0001.html>


More information about the asterisk-code-review mailing list