[Asterisk-code-review] astobj2: Add backtrace to log bad ao2. (asterisk[master])

Joshua Colp asteriskteam at digium.com
Tue Oct 4 11:16:59 CDT 2016


Joshua Colp has submitted this change and it was merged.

Change subject: astobj2: Add backtrace to log_bad_ao2.
......................................................................


astobj2: Add backtrace to log_bad_ao2.

* Compile __ast_assert_failed unconditionally.
* Use __ast_assert_failed to log messages from log_bad_ao2
* Remove calls to ast_assert(0) that happen after log_bad_ao2 was run.

Change-Id: I48f1af44b2718ad74a421ff75cb6397b924a9751
---
M include/asterisk/utils.h
M main/astobj2.c
M main/astobj2_container.c
M main/astobj2_hash.c
M main/astobj2_rbtree.c
M main/utils.c
6 files changed, 43 insertions(+), 43 deletions(-)

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



diff --git a/include/asterisk/utils.h b/include/asterisk/utils.h
index f9608f5..2378c69 100644
--- a/include/asterisk/utils.h
+++ b/include/asterisk/utils.h
@@ -848,8 +848,10 @@
 #define DO_CRASH_NORETURN
 #endif
 
+void DO_CRASH_NORETURN __ast_assert_failed(int condition, const char *condition_str,
+	const char *file, int line, const char *function);
+
 #ifdef AST_DEVMODE
-void DO_CRASH_NORETURN __ast_assert_failed(int condition, const char *condition_str, const char *file, int line, const char *function);
 #define ast_assert(a) _ast_assert(a, # a, __FILE__, __LINE__, __PRETTY_FUNCTION__)
 static void force_inline _ast_assert(int condition, const char *condition_str, const char *file, int line, const char *function)
 {
diff --git a/main/astobj2.c b/main/astobj2.c
index 604e6a1..72bfcc8 100644
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -178,22 +178,17 @@
 void log_bad_ao2(void *user_data, const char *file, int line, const char *func)
 {
 	struct astobj2 *p;
+	char bad_magic[100];
 
 	if (!user_data) {
-		ast_log(__LOG_ERROR, file, line, func, "user_data is NULL\n");
+		__ast_assert_failed(0, "user_data is NULL", file, line, func);
 		return;
 	}
 
 	p = INTERNAL_OBJ(user_data);
-	if (p->priv_data.magic) {
-		ast_log(__LOG_ERROR, file, line, func,
-			"bad magic number 0x%x for object %p\n",
-			p->priv_data.magic, user_data);
-	} else {
-		ast_log(__LOG_ERROR, file, line, func,
-			"bad magic number for object %p. Object is likely destroyed.\n",
-			user_data);
-	}
+	snprintf(bad_magic, sizeof(bad_magic), "bad magic number 0x%x for object %p",
+		p->priv_data.magic, user_data);
+	__ast_assert_failed(0, bad_magic, file, line, func);
 }
 
 int __ao2_lock(void *user_data, enum ao2_lock_req lock_how, const char *file, const char *func, int line, const char *var)
@@ -205,7 +200,6 @@
 	int res = 0;
 
 	if (obj == NULL) {
-		ast_assert(0);
 		return -1;
 	}
 
@@ -269,7 +263,6 @@
 	int current_value;
 
 	if (obj == NULL) {
-		ast_assert(0);
 		return -1;
 	}
 
@@ -323,7 +316,6 @@
 	int res = 0;
 
 	if (obj == NULL) {
-		ast_assert(0);
 		return -1;
 	}
 
@@ -454,7 +446,6 @@
 	obj = INTERNAL_OBJ_CHECK(user_data);
 
 	if (obj == NULL) {
-		ast_assert(0);
 		return NULL;
 	}
 
@@ -486,7 +477,6 @@
 				user_data, delta, ast_get_tid(), file, line, func, tag ?: "");
 			fflush(ref_log);
 		}
-		ast_assert(0);
 		return -1;
 	}
 
diff --git a/main/astobj2_container.c b/main/astobj2_container.c
index c00da9f..15fd412 100644
--- a/main/astobj2_container.c
+++ b/main/astobj2_container.c
@@ -101,10 +101,13 @@
 	struct ao2_container_node *node;
 
 	if (!__is_ao2_object(obj_new, file, line, func)
-		|| !__is_ao2_object(self, file, line, func)
-		|| !self->v_table || !self->v_table->new_node || !self->v_table->insert) {
+		|| !__is_ao2_object(self, file, line, func)) {
+		return 0;
+	}
+
+	if (!self->v_table || !self->v_table->new_node || !self->v_table->insert) {
 		/* Sanity checks. */
-		ast_assert(0);
+		__ast_assert_failed(0, "invalid container v_table", file, line, func);
 		return 0;
 	}
 
@@ -176,7 +179,6 @@
 {
 	if (!__is_ao2_object(user_data, file, line, func)) {
 		/* Sanity checks. */
-		ast_assert(0);
 		return NULL;
 	}
 
@@ -241,10 +243,14 @@
 	struct ao2_container *multi_container = NULL;
 	struct ao2_iterator *multi_iterator = NULL;
 
-	if (!__is_ao2_object(self, file, line, func) || !self->v_table
+	if (!__is_ao2_object(self, file, line, func)) {
+		return NULL;
+	}
+
+	if (!self->v_table
 		|| !self->v_table->traverse_first || !self->v_table->traverse_next) {
 		/* Sanity checks. */
-		ast_assert(0);
+		__ast_assert_failed(0, "invalid container v_table", file, line, func);
 		return NULL;
 	}
 
@@ -451,20 +457,13 @@
 void ao2_iterator_restart(struct ao2_iterator *iter)
 {
 	if (!is_ao2_object(iter->c)) {
-		ast_log(LOG_ERROR, "Iterator container is not valid.\n");
-		ast_assert(0);
+		/* Sanity check. */
 		return;
 	}
 
 	/* Release the last container node reference if we have one. */
 	if (iter->last_node) {
 		enum ao2_lock_req orig_lock;
-
-		if (!is_ao2_object(iter->c)) {
-			/* Sanity check. */
-			ast_assert(0);
-			return;
-		}
 
 		/*
 		 * Do a read lock in case the container node unref does not
@@ -521,10 +520,13 @@
 	struct ao2_container_node *node;
 	void *ret;
 
-	if (!__is_ao2_object(iter->c, file, line, func)
-		|| !iter->c->v_table || !iter->c->v_table->iterator_next) {
+	if (!__is_ao2_object(iter->c, file, line, func)) {
+		return NULL;
+	}
+
+	if (!iter->c->v_table || !iter->c->v_table->iterator_next) {
 		/* Sanity checks. */
-		ast_assert(0);
+		__ast_assert_failed(0, "invalid iterator container v_table", file, line, func);
 		return NULL;
 	}
 
@@ -661,12 +663,16 @@
 	int failed;
 
 	/* Create the clone container with the same properties as the original. */
-	if (!__is_ao2_object(orig, file, line, func)
-		|| !orig->v_table || !orig->v_table->alloc_empty_clone) {
-		/* Sanity checks. */
-		ast_assert(0);
+	if (!__is_ao2_object(orig, file, line, func)) {
 		return NULL;
 	}
+
+	if (!orig->v_table || !orig->v_table->alloc_empty_clone) {
+		/* Sanity checks. */
+		__ast_assert_failed(0, "invalid container v_table", file, line, func);
+		return NULL;
+	}
+
 	clone = orig->v_table->alloc_empty_clone(orig, tag, file, line, func);
 	if (!clone) {
 		return NULL;
diff --git a/main/astobj2_hash.c b/main/astobj2_hash.c
index 072cd18..eee90f5 100644
--- a/main/astobj2_hash.c
+++ b/main/astobj2_hash.c
@@ -166,7 +166,9 @@
 		 * same node.
 		 */
 		my_container = (struct ao2_container_hash *) doomed->common.my_container;
-		ast_assert(is_ao2_object(my_container));
+#ifdef AST_DEVMODE
+		is_ao2_object(my_container);
+#endif
 
 		__adjust_lock(my_container, AO2_LOCK_REQ_WRLOCK, 1);
 
diff --git a/main/astobj2_rbtree.c b/main/astobj2_rbtree.c
index 03efd88..1760938 100644
--- a/main/astobj2_rbtree.c
+++ b/main/astobj2_rbtree.c
@@ -858,7 +858,9 @@
 		 * same node.
 		 */
 		my_container = (struct ao2_container_rbtree *) doomed->common.my_container;
-		ast_assert(is_ao2_object(my_container));
+#ifdef AST_DEVMODE
+		is_ao2_object(my_container);
+#endif
 
 		__adjust_lock(my_container, AO2_LOCK_REQ_WRLOCK, 1);
 
diff --git a/main/utils.c b/main/utils.c
index c8ede91..fc7aa30 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -2467,17 +2467,16 @@
 #endif	/* defined(DO_CRASH) */
 }
 
-#if defined(AST_DEVMODE)
 void DO_CRASH_NORETURN __ast_assert_failed(int condition, const char *condition_str, const char *file, int line, const char *function)
 {
 	/*
 	 * Attempt to put it into the logger, but hope that at least
 	 * someone saw the message on stderr ...
 	 */
-	ast_log(__LOG_ERROR, file, line, function, "FRACK!, Failed assertion %s (%d)\n",
-		condition_str, condition);
 	fprintf(stderr, "FRACK!, Failed assertion %s (%d) at line %d in %s of %s\n",
 		condition_str, condition, line, function, file);
+	ast_log(__LOG_ERROR, file, line, function, "FRACK!, Failed assertion %s (%d)\n",
+		condition_str, condition);
 
 	/* Generate a backtrace for the assert */
 	ast_log_backtrace();
@@ -2490,7 +2489,6 @@
 	usleep(1);
 	ast_do_crash();
 }
-#endif	/* defined(AST_DEVMODE) */
 
 char *ast_eid_to_str(char *s, int maxlen, struct ast_eid *eid)
 {

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I48f1af44b2718ad74a421ff75cb6397b924a9751
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list