[Asterisk-code-review] manager.c: Eliminate most RAII VAR usage. (asterisk[master])

Joshua Colp asteriskteam at digium.com
Tue Apr 26 04:57:36 CDT 2016


Joshua Colp has submitted this change and it was merged.

Change subject: manager.c: Eliminate most RAII_VAR usage.
......................................................................


manager.c: Eliminate most RAII_VAR usage.

* Made ast_manager_event_blob_create() not allocate the ao2 event object
with a lock as it is not needed.

Change-Id: I8e11bfedd22c21316012e0b9dd79f5918f644b7c
---
M main/manager.c
1 file changed, 43 insertions(+), 31 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Joshua Colp: Looks good to me, but someone else must approve; Verified
  George Joseph: Looks good to me, but someone else must approve



diff --git a/main/manager.c b/main/manager.c
index 74e9533..0cef582 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -1690,17 +1690,17 @@
 static void manager_default_msg_cb(void *data, struct stasis_subscription *sub,
 				    struct stasis_message *message)
 {
-	RAII_VAR(struct ast_manager_event_blob *, ev, NULL, ao2_cleanup);
+	struct ast_manager_event_blob *ev;
 
 	ev = stasis_message_to_ami(message);
-
-	if (ev == NULL) {
-		/* Not and AMI message; disregard */
+	if (!ev) {
+		/* Not an AMI message; disregard */
 		return;
 	}
 
 	manager_event(ev->event_flags, ev->manager_event, "%s",
 		ev->extra_fields);
+	ao2_ref(ev, -1);
 }
 
 static void manager_generic_msg_cb(void *data, struct stasis_subscription *sub,
@@ -1710,7 +1710,7 @@
 	int class_type = ast_json_integer_get(ast_json_object_get(payload->json, "class_type"));
 	const char *type = ast_json_string_get(ast_json_object_get(payload->json, "type"));
 	struct ast_json *event = ast_json_object_get(payload->json, "event");
-	RAII_VAR(struct ast_str *, event_buffer, NULL, ast_free);
+	struct ast_str *event_buffer;
 
 	event_buffer = ast_manager_str_from_json_object(event, NULL);
 	if (!event_buffer) {
@@ -1718,6 +1718,7 @@
 		return;
 	}
 	manager_event(class_type, type, "%s", ast_str_buffer(event_buffer));
+	ast_free(event_buffer);
 }
 
 void ast_manager_publish_event(const char *type, int class_type, struct ast_json *obj)
@@ -4716,7 +4717,7 @@
 	const char *name = astman_get_header(m, "Channel");
 	const char *exten = astman_get_header(m, "Exten");
 	const char *context = astman_get_header(m, "Context");
-	RAII_VAR(struct ast_channel *, chan, NULL, ao2_cleanup);
+	struct ast_channel *chan;
 
 	if (ast_strlen_zero(name)) {
 		astman_send_error(s, m, "No channel specified");
@@ -4753,6 +4754,7 @@
 		break;
 	}
 
+	ast_channel_unref(chan);
 	return 0;
 }
 
@@ -5931,7 +5933,7 @@
 	const char *actionid = astman_get_header(m, "ActionID");
 	char idText[256];
 	int numchans = 0;
-	RAII_VAR(struct ao2_container *, channels, NULL, ao2_cleanup);
+	struct ao2_container *channels;
 	struct ao2_iterator it_chans;
 	struct stasis_message *msg;
 
@@ -5941,7 +5943,8 @@
 		idText[0] = '\0';
 	}
 
-	if (!(channels = stasis_cache_dump(ast_channel_cache_by_name(), ast_channel_snapshot_type()))) {
+	channels = stasis_cache_dump(ast_channel_cache_by_name(), ast_channel_snapshot_type());
+	if (!channels) {
 		astman_send_error(s, m, "Could not get cached channels");
 		return 0;
 	}
@@ -5993,6 +5996,7 @@
 	astman_send_list_complete_start(s, m, "CoreShowChannelsComplete", numchans);
 	astman_send_list_complete_end(s);
 
+	ao2_ref(channels, -1);
 	return 0;
 }
 
@@ -6623,11 +6627,10 @@
 
 static void append_channel_vars(struct ast_str **pbuf, struct ast_channel *chan)
 {
-	RAII_VAR(struct varshead *, vars, NULL, ao2_cleanup);
+	struct varshead *vars;
 	struct ast_var_t *var;
 
 	vars = ast_channel_get_manager_vars(chan);
-
 	if (!vars) {
 		return;
 	}
@@ -6635,6 +6638,7 @@
 	AST_LIST_TRAVERSE(vars, var, entries) {
 		ast_str_append(pbuf, 0, "ChanVariable(%s): %s=%s\r\n", ast_channel_name(chan), var->name, var->value);
 	}
+	ao2_ref(vars, -1);
 }
 
 /* XXX see if can be moved inside the function */
@@ -6646,8 +6650,6 @@
 	const char *fmt, ...)
 {
 	RAII_VAR(struct ao2_container *, sessions, ao2_global_obj_ref(mgr_sessions), ao2_cleanup);
-	struct mansession_session *session;
-	struct manager_custom_hook *hook;
 	struct ast_str *auth = ast_str_alloca(MAX_AUTH_PERM_STRING);
 	const char *cat_str;
 	va_list ap;
@@ -6659,33 +6661,39 @@
 		return 0;
 	}
 
-	if (!(buf = ast_str_thread_get(&manager_event_buf, MANAGER_EVENT_BUF_INITSIZE))) {
+	buf = ast_str_thread_get(&manager_event_buf, MANAGER_EVENT_BUF_INITSIZE);
+	if (!buf) {
 		return -1;
 	}
 
 	cat_str = authority_to_str(category, &auth);
 	ast_str_set(&buf, 0,
-			"Event: %s\r\nPrivilege: %s\r\n",
-			 event, cat_str);
+		"Event: %s\r\n"
+		"Privilege: %s\r\n",
+		event, cat_str);
 
 	if (timestampevents) {
 		now = ast_tvnow();
 		ast_str_append(&buf, 0,
-				"Timestamp: %ld.%06lu\r\n",
-				 (long)now.tv_sec, (unsigned long) now.tv_usec);
+			"Timestamp: %ld.%06lu\r\n",
+			(long)now.tv_sec, (unsigned long) now.tv_usec);
 	}
 	if (manager_debug) {
 		static int seq;
+
 		ast_str_append(&buf, 0,
-				"SequenceNumber: %d\r\n",
-				 ast_atomic_fetchadd_int(&seq, 1));
+			"SequenceNumber: %d\r\n",
+			ast_atomic_fetchadd_int(&seq, 1));
 		ast_str_append(&buf, 0,
-				"File: %s\r\nLine: %d\r\nFunc: %s\r\n", file, line, func);
+			"File: %s\r\n"
+			"Line: %d\r\n"
+			"Func: %s\r\n",
+			file, line, func);
 	}
 	if (!ast_strlen_zero(ast_config_AST_SYSTEM_NAME)) {
 		ast_str_append(&buf, 0,
-				"SystemName: %s\r\n",
-				 ast_config_AST_SYSTEM_NAME);
+			"SystemName: %s\r\n",
+			ast_config_AST_SYSTEM_NAME);
 	}
 
 	va_start(ap, fmt);
@@ -6701,9 +6709,11 @@
 
 	/* Wake up any sleeping sessions */
 	if (sessions) {
-		struct ao2_iterator i;
-		i = ao2_iterator_init(sessions, 0);
-		while ((session = ao2_iterator_next(&i))) {
+		struct ao2_iterator iter;
+		struct mansession_session *session;
+
+		iter = ao2_iterator_init(sessions, 0);
+		while ((session = ao2_iterator_next(&iter))) {
 			ao2_lock(session);
 			if (session->waiting_thread != AST_PTHREADT_NULL) {
 				pthread_kill(session->waiting_thread, SIGURG);
@@ -6718,10 +6728,12 @@
 			ao2_unlock(session);
 			unref_mansession(session);
 		}
-		ao2_iterator_destroy(&i);
+		ao2_iterator_destroy(&iter);
 	}
 
 	if (category != EVENT_FLAG_SHUTDOWN && !AST_RWLIST_EMPTY(&manager_hooks)) {
+		struct manager_custom_hook *hook;
+
 		AST_RWLIST_RDLOCK(&manager_hooks);
 		AST_RWLIST_TRAVERSE(&manager_hooks, hook, list) {
 			hook->helper(category, event, ast_str_buffer(buf));
@@ -9217,6 +9229,7 @@
 static void manager_event_blob_dtor(void *obj)
 {
 	struct ast_manager_event_blob *ev = obj;
+
 	ast_string_field_free_memory(ev);
 }
 
@@ -9228,18 +9241,19 @@
 	const char *extra_fields_fmt,
 	...)
 {
-	RAII_VAR(struct ast_manager_event_blob *, ev, NULL, ao2_cleanup);
+	struct ast_manager_event_blob *ev;
 	va_list argp;
 
 	ast_assert(extra_fields_fmt != NULL);
 	ast_assert(manager_event != NULL);
 
-	ev = ao2_alloc(sizeof(*ev), manager_event_blob_dtor);
+	ev = ao2_alloc_options(sizeof(*ev), manager_event_blob_dtor, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!ev) {
 		return NULL;
 	}
 
 	if (ast_string_field_init(ev, 20)) {
+		ao2_ref(ev, -1);
 		return NULL;
 	}
 
@@ -9247,10 +9261,8 @@
 	ev->event_flags = event_flags;
 
 	va_start(argp, extra_fields_fmt);
-	ast_string_field_ptr_build_va(ev, &ev->extra_fields, extra_fields_fmt,
-				      argp);
+	ast_string_field_ptr_build_va(ev, &ev->extra_fields, extra_fields_fmt, argp);
 	va_end(argp);
 
-	ao2_ref(ev, +1);
 	return ev;
 }

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8e11bfedd22c21316012e0b9dd79f5918f644b7c
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list