[Asterisk-code-review] stringfields: Update extended string fields for master only. (asterisk[master])

George Joseph asteriskteam at digium.com
Wed Apr 13 13:43:48 CDT 2016


George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/2581

Change subject: stringfields:  Update extended string fields for master only.
......................................................................

stringfields:  Update extended string fields for master only.

In 13, the new ast_string_field_header structure had to be dynamically
allocated and assigned to a pointer in ast_string_field_mgr to preserve ABI
compatability.  In master, it can be converted to being a structure-in-place in
ast_string_field_mgr to eliminate the extra alloc and free calls.

Change-Id: Ia97c5345eec68717a15dc16fe2e6746ff2a926f4
---
M include/asterisk/stringfields.h
M main/stringfields.c
M tests/test_stringfields.c
3 files changed, 27 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/81/2581/1

diff --git a/include/asterisk/stringfields.h b/include/asterisk/stringfields.h
index c24424b..a222dae 100644
--- a/include/asterisk/stringfields.h
+++ b/include/asterisk/stringfields.h
@@ -236,7 +236,7 @@
 */
 struct ast_string_field_mgr {
 	ast_string_field last_alloc;			/*!< the last field allocated */
-	struct ast_string_field_header *header;	/*!< pointer to the header */
+	struct ast_string_field_header header;	/*!< header */
 #if defined(__AST_DEBUG_MALLOC)
 	const char *owner_file;				/*!< filename of owner */
 	const char *owner_func;				/*!< function name of owner */
@@ -407,10 +407,10 @@
 #define ast_string_field_init_extended(x, field) \
 ({ \
 	int __res__ = -1; \
-	if (((void *)(x)) != NULL && (x)->__field_mgr.header != NULL) { \
+	if (((void *)(x)) != NULL) { \
 		ast_string_field *non_const = (ast_string_field *)&(x)->field; \
 		*non_const = __ast_string_field_empty; \
-		__res__ = AST_VECTOR_APPEND(&(x)->__field_mgr.header->string_fields, non_const); \
+		__res__ = AST_VECTOR_APPEND(&(x)->__field_mgr.header.string_fields, non_const); \
 	} \
 	__res__; \
 })
@@ -613,8 +613,8 @@
 ({ \
 	int __res__ = -1; \
 	if (((void *)(instance1)) != NULL && ((void *)(instance2)) != NULL) { \
-		__res__ = __ast_string_fields_cmp(&(instance1)->__field_mgr.header->string_fields, \
-			&(instance2)->__field_mgr.header->string_fields); \
+		__res__ = __ast_string_fields_cmp(&(instance1)->__field_mgr.header.string_fields, \
+			&(instance2)->__field_mgr.header.string_fields); \
 	} \
 	__res__; \
 })
diff --git a/main/stringfields.c b/main/stringfields.c
index 67dd06c..8b72af0 100644
--- a/main/stringfields.c
+++ b/main/stringfields.c
@@ -118,29 +118,23 @@
 	struct ast_string_field_pool *cur = NULL;
 	struct ast_string_field_pool *preserve = NULL;
 
-	if (!mgr->header) {
-		return -1;
-	}
-
 	/* reset all the fields regardless of cleanup type */
-	AST_VECTOR_CALLBACK_VOID(&mgr->header->string_fields, reset_field);
+	AST_VECTOR_CALLBACK_VOID(&mgr->header.string_fields, reset_field);
 
 	switch (cleanup_type) {
 	case AST_STRINGFIELD_DESTROY:
-		AST_VECTOR_FREE(&mgr->header->string_fields);
+		AST_VECTOR_FREE(&mgr->header.string_fields);
 
-		if (mgr->header->embedded_pool) { /* ALWAYS preserve the embedded pool if there is one */
-			preserve = mgr->header->embedded_pool;
+		if (mgr->header.embedded_pool) { /* ALWAYS preserve the embedded pool if there is one */
+			preserve = mgr->header.embedded_pool;
 			preserve->used = preserve->active = 0;
 		}
 
-		ast_free(mgr->header);
-		mgr->header = NULL;
 		break;
 	case AST_STRINGFIELD_RESET:
 		/* Preserve the embedded pool if there is one, otherwise the last pool */
-		if (mgr->header->embedded_pool) {
-			preserve = mgr->header->embedded_pool;
+		if (mgr->header.embedded_pool) {
+			preserve = mgr->header.embedded_pool;
 		} else {
 			if (*pool_head == NULL) {
 				ast_log(LOG_WARNING, "trying to reset empty pool\n");
@@ -202,27 +196,19 @@
 	mgr->owner_line = lineno;
 #endif
 
-	if (!(mgr->header = calloc_wrapper(1, sizeof(*mgr->header), file, lineno, func))) {
-		return -1;
-	}
-
-	if (AST_VECTOR_INIT(&mgr->header->string_fields, initial_vector_size)) {
-		ast_free(mgr->header);
-		mgr->header = NULL;
+	if (AST_VECTOR_INIT(&mgr->header.string_fields, initial_vector_size)) {
 		return -1;
 	}
 
 	while ((struct ast_string_field_mgr *) p != mgr) {
-		AST_VECTOR_APPEND(&mgr->header->string_fields, p);
+		AST_VECTOR_APPEND(&mgr->header.string_fields, p);
 		*p++ = __ast_string_field_empty;
 	}
 
 	*pool_head = NULL;
-	mgr->header->embedded_pool = NULL;
+	mgr->header.embedded_pool = NULL;
 	if (add_string_pool(mgr, pool_head, needed, file, lineno, func)) {
-		AST_VECTOR_FREE(&mgr->header->string_fields);
-		ast_free(mgr->header);
-		mgr->header = NULL;
+		AST_VECTOR_FREE(&mgr->header.string_fields);
 		return -1;
 	}
 
@@ -428,33 +414,22 @@
 
 	mgr = allocation + field_mgr_offset;
 
-	/*
-	 * The header is calloced in __ast_string_field_init so it also gets calloced here
-	 * so __ast_string_fields_free_memory can always just free mgr->header.
-	 */
-	mgr->header = calloc_wrapper(1, sizeof(*mgr->header), file, lineno, func);
-	if (!mgr->header) {
-		ast_free(allocation);
-		return NULL;
-	}
-
 	pool = allocation + struct_size;
 	pool_head = allocation + field_mgr_pool_offset;
 	p = (const char **) pool_head + 1;
 	initial_vector_size = ((size_t) (((char *)mgr) - ((char *)p))) / sizeof(*p);
 
-	if (AST_VECTOR_INIT(&mgr->header->string_fields, initial_vector_size)) {
-		ast_free(mgr->header);
+	if (AST_VECTOR_INIT(&mgr->header.string_fields, initial_vector_size)) {
 		ast_free(allocation);
 		return NULL;
 	}
 
 	while ((struct ast_string_field_mgr *) p != mgr) {
-		AST_VECTOR_APPEND(&mgr->header->string_fields, p);
+		AST_VECTOR_APPEND(&mgr->header.string_fields, p);
 		*p++ = __ast_string_field_empty;
 	}
 
-	mgr->header->embedded_pool = pool;
+	mgr->header.embedded_pool = pool;
 	*pool_head = pool;
 	pool->size = size_to_alloc - struct_size - sizeof(*pool);
 #if defined(__AST_DEBUG_MALLOC)
@@ -487,8 +462,8 @@
 	struct ast_string_field_mgr *copy_mgr, struct ast_string_field_mgr *orig_mgr)
 {
 	int i;
-	struct ast_string_field_vector *dest = &(copy_mgr->header->string_fields);
-	struct ast_string_field_vector *src = &(orig_mgr->header->string_fields);
+	struct ast_string_field_vector *dest = &(copy_mgr->header.string_fields);
+	struct ast_string_field_vector *src = &(orig_mgr->header.string_fields);
 
 	ast_assert(AST_VECTOR_SIZE(dest) == AST_VECTOR_SIZE(src));
 
diff --git a/tests/test_stringfields.c b/tests/test_stringfields.c
index e57f8d7..0494e1e 100644
--- a/tests/test_stringfields.c
+++ b/tests/test_stringfields.c
@@ -369,7 +369,7 @@
 	ast_string_field_ptr_set_by_fields(inst2->__field_mgr_pool, inst2->__field_mgr, &inst2->string1, "foo");
 	inst2->foo = 2;
 
-	if (inst3->__field_mgr.header->embedded_pool->prev) {
+	if (inst3->__field_mgr.header.embedded_pool->prev) {
 		ast_test_status_update(test, "Structure 3 embedded pool should not have a previous pool!\n");
 		res = AST_TEST_FAIL;
 		goto error;
@@ -377,13 +377,13 @@
 
 	ast_string_field_set(inst3, string1, "foo");
 
-	if (inst3->__field_mgr.header->embedded_pool != inst3->__field_mgr_pool) {
+	if (inst3->__field_mgr.header.embedded_pool != inst3->__field_mgr_pool) {
 		ast_test_status_update(test, "Structure 3 embedded pool should have been the current pool!\n");
 		res = AST_TEST_FAIL;
 		goto error;
 	}
 
-	if (inst3->__field_mgr.header->embedded_pool->prev) {
+	if (inst3->__field_mgr.header.embedded_pool->prev) {
 		ast_test_status_update(test, "Structure 3 embedded pool should not have a previous pool!\n");
 		res = AST_TEST_FAIL;
 		goto error;
@@ -395,13 +395,13 @@
 	ast_string_field_set(inst3, string2, "baz 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890 1234567890");
 	inst3->foo = 3;
 
-	if (inst3->__field_mgr_pool == inst3->__field_mgr.header->embedded_pool) {
+	if (inst3->__field_mgr_pool == inst3->__field_mgr.header.embedded_pool) {
 		ast_test_status_update(test, "Structure 3 embedded pool should not have been the current pool!\n");
 		res = AST_TEST_FAIL;
 		goto error;
 	}
 
-	if (inst3->__field_mgr.header->embedded_pool != inst3->__field_mgr_pool->prev) {
+	if (inst3->__field_mgr.header.embedded_pool != inst3->__field_mgr_pool->prev) {
 		ast_test_status_update(test, "Structure 3 embedded pool should be the current pool's previous!\n");
 		res = AST_TEST_FAIL;
 		goto error;
@@ -484,7 +484,7 @@
 		ast_test_status_update(test, "Structures 1/2 are the same (empty) as expected.\n");
 	}
 
-	if (inst4->__field_mgr.header->embedded_pool != inst4->__field_mgr_pool) {
+	if (inst4->__field_mgr.header.embedded_pool != inst4->__field_mgr_pool) {
 		ast_test_status_update(test, "Structure 4 embedded pool should have been the current pool!\n");
 		res = AST_TEST_FAIL;
 		goto error;
@@ -492,7 +492,7 @@
 		ast_test_status_update(test, "Structure 4 embedded pool is the current pool as expected.\n");
 	}
 
-	if (inst4->__field_mgr.header->embedded_pool->prev) {
+	if (inst4->__field_mgr.header.embedded_pool->prev) {
 		ast_test_status_update(test, "Structure 4 embedded pool should not have a previous pool!\n");
 		res = AST_TEST_FAIL;
 		goto error;

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia97c5345eec68717a15dc16fe2e6746ff2a926f4
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>



More information about the asterisk-code-review mailing list