[svn-commits] dlee: branch dlee/endpoints r387789 - in /team/dlee/endpoints: channels/ incl...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Mon May 6 16:43:56 CDT 2013


Author: dlee
Date: Mon May  6 16:43:54 2013
New Revision: 387789

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=387789
Log:
Address review feedback.

* Removed some needless temp variables
* Comment fixes
* Removed an unnecessary lock
* Removed unnecessary debug statements

Modified:
    team/dlee/endpoints/channels/chan_sip.c
    team/dlee/endpoints/include/asterisk/endpoints.h
    team/dlee/endpoints/main/endpoints.c
    team/dlee/endpoints/main/stasis_cache.c
    team/dlee/endpoints/main/stasis_endpoints.c
    team/dlee/endpoints/tests/test_stasis_endpoints.c

Modified: team/dlee/endpoints/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/endpoints/channels/chan_sip.c?view=diff&rev=387789&r1=387788&r2=387789
==============================================================================
--- team/dlee/endpoints/channels/chan_sip.c (original)
+++ team/dlee/endpoints/channels/chan_sip.c Mon May  6 16:43:54 2013
@@ -8017,9 +8017,7 @@
 	}
 
 	if (i->relatedpeer) {
-		int r;
-		r = ast_endpoint_add_channel(i->relatedpeer->endpoint, tmp);
-		if (r != 0) {
+		if (ast_endpoint_add_channel(i->relatedpeer->endpoint, tmp)) {
 			ast_channel_unref(tmp);
 			sip_pvt_lock(i);
 			return NULL;

Modified: team/dlee/endpoints/include/asterisk/endpoints.h
URL: http://svnview.digium.com/svn/asterisk/team/dlee/endpoints/include/asterisk/endpoints.h?view=diff&rev=387789&r1=387788&r2=387789
==============================================================================
--- team/dlee/endpoints/include/asterisk/endpoints.h (original)
+++ team/dlee/endpoints/include/asterisk/endpoints.h Mon May  6 16:43:54 2013
@@ -33,7 +33,7 @@
  * This file defines \ref ast_endpoint as a seperate object, which channel
  * drivers may use to expose their concept of an endpoint. As the channel driver
  * creates channels, it can use ast_endpoint_add_channel() to associate channels
- * to the endpoint. This updated the endpoint appropriately, and forwards all of
+ * to the endpoint. This updates the endpoint appropriately, and forwards all of
  * the channel's events to the endpoint's topic.
  *
  * In order to avoid excessive locking on the endpoint object itself, the
@@ -57,6 +57,13 @@
 	AST_ENDPOINT_ONLINE,
 };
 
+/*!
+ * \brief Returns a string representation of the given endpoint state.
+ *
+ * \param state Endpoint state.
+ * \return String representation of \a state.
+ * \return \c "?" if \a state isn't in \ref ast_endpoint_state.
+ */
 const char *ast_endpoint_state_to_string(enum ast_endpoint_state state);
 
 /*!

Modified: team/dlee/endpoints/main/endpoints.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/endpoints/main/endpoints.c?view=diff&rev=387789&r1=387788&r2=387789
==============================================================================
--- team/dlee/endpoints/main/endpoints.c (original)
+++ team/dlee/endpoints/main/endpoints.c Mon May  6 16:43:54 2013
@@ -50,7 +50,12 @@
 		);
 	/*! Endpoint's current state */
 	enum ast_endpoint_state state;
-	/*! Max channels for this endpoint. -1 means unlimited or unknown. */
+	/*!
+	 * \brief Max channels for this endpoint. -1 means unlimited or unknown.
+	 *
+	 * Note that this simply documents the limits of an endpoint, and does
+	 * nothing to try to enforce the limit.
+	 */
 	int max_channels;
 	/*! Topic for this endpoint's messages */
 	struct stasis_topic *topic;
@@ -180,8 +185,6 @@
 		return NULL;
 	}
 
-	ast_debug(3, "%s(%s, %s)\n", __func__, tech, resource);
-
 	endpoint = ao2_alloc(sizeof(*endpoint), endpoint_dtor);
 	if (!endpoint) {
 		return NULL;
@@ -198,7 +201,10 @@
 	ast_string_field_set(endpoint, resource, resource);
 	ast_string_field_build(endpoint, id, "%s/%s", tech, resource);
 
-	endpoint->channel_ids = ast_str_container_alloc(ENDPOINT_BUCKETS);
+	/* All access to channel_ids should be covered by the endpoint's
+	 * lock; no extra lock needed. */
+	endpoint->channel_ids = ast_str_container_alloc_options(
+		AO2_ALLOC_OPT_LOCK_NOLOCK, ENDPOINT_BUCKETS);
 	if (!endpoint->channel_ids) {
 		return NULL;
 	}
@@ -270,7 +276,6 @@
 	enum ast_endpoint_state state)
 {
 	ast_assert(endpoint != NULL);
-	ast_debug(3, "%s(%s, %d)\n", __func__, endpoint->id, state);
 	ao2_lock(endpoint);
 	endpoint->state = state;
 	ao2_unlock(endpoint);
@@ -281,7 +286,6 @@
 	int max_channels)
 {
 	ast_assert(endpoint != NULL);
-	ast_debug(3, "%s(%s, %d)\n", __func__, endpoint->id, max_channels);
 	ao2_lock(endpoint);
 	endpoint->max_channels = max_channels;
 	ao2_unlock(endpoint);

Modified: team/dlee/endpoints/main/stasis_cache.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/endpoints/main/stasis_cache.c?view=diff&rev=387789&r1=387788&r2=387789
==============================================================================
--- team/dlee/endpoints/main/stasis_cache.c (original)
+++ team/dlee/endpoints/main/stasis_cache.c Mon May  6 16:43:54 2013
@@ -367,7 +367,7 @@
 		} else {
 			/* While this could be a problem, it's very likely to
 			 * happen with message forwarding */
-			ast_log(LOG_DEBUG,
+			ast_debug(1,
 				"Attempting to remove an item from the cache that isn't there: %s %s\n",
 				stasis_message_type_name(clear->type), clear->id);
 		}

Modified: team/dlee/endpoints/main/stasis_endpoints.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/endpoints/main/stasis_endpoints.c?view=diff&rev=387789&r1=387788&r2=387789
==============================================================================
--- team/dlee/endpoints/main/stasis_endpoints.c (original)
+++ team/dlee/endpoints/main/stasis_endpoints.c Mon May  6 16:43:54 2013
@@ -120,7 +120,6 @@
 {
 	RAII_VAR(struct ast_json *, json, NULL, ast_json_unref);
 	struct ast_json *channel_array;
-	int v;
 	int i;
 
 	json = ast_json_pack("{s: s, s: s, s: s, s: []}",
@@ -134,9 +133,9 @@
 	}
 
 	if (snapshot->max_channels != -1) {
-		v = ast_json_object_set(json, "max_channels",
+		int res = ast_json_object_set(json, "max_channels",
 			ast_json_integer_create(snapshot->max_channels));
-		if (v != 0) {
+		if (res != 0) {
 			return NULL;
 		}
 	}
@@ -144,10 +143,10 @@
 	channel_array = ast_json_object_get(json, "channels");
 	ast_assert(channel_array != NULL);
 	for (i = 0; i < snapshot->num_channels; ++i) {
-		v = ast_json_array_append(channel_array,
+		int res = ast_json_array_append(channel_array,
 			ast_json_stringf("channel:%s",
 				snapshot->channel_ids[i]));
-		if (v != 0) {
+		if (res != 0) {
 			return NULL;
 		}
 	}

Modified: team/dlee/endpoints/tests/test_stasis_endpoints.c
URL: http://svnview.digium.com/svn/asterisk/team/dlee/endpoints/tests/test_stasis_endpoints.c?view=diff&rev=387789&r1=387788&r2=387789
==============================================================================
--- team/dlee/endpoints/tests/test_stasis_endpoints.c (original)
+++ team/dlee/endpoints/tests/test_stasis_endpoints.c Mon May  6 16:43:54 2013
@@ -76,7 +76,7 @@
 	return 0 == strcmp(name, snapshot->resource);
 }
 
-AST_TEST_DEFINE(state_change)
+AST_TEST_DEFINE(state_changes)
 {
 	RAII_VAR(struct ast_endpoint *, uut, NULL, ast_endpoint_shutdown);
 	RAII_VAR(struct ast_channel *, chan, NULL, safe_channel_hangup);
@@ -286,7 +286,7 @@
 static int unload_module(void)
 {
 	AST_TEST_UNREGISTER(state_changes);
-	AST_TEST_UNREGISTER(channel_cache_clear);
+	AST_TEST_UNREGISTER(cache_clear);
 	AST_TEST_UNREGISTER(channel_messages);
 	return 0;
 }
@@ -294,7 +294,7 @@
 static int load_module(void)
 {
 	AST_TEST_REGISTER(state_changes);
-	AST_TEST_REGISTER(channel_cache_clear);
+	AST_TEST_REGISTER(cache_clear);
 	AST_TEST_REGISTER(channel_messages);
 	return AST_MODULE_LOAD_SUCCESS;
 }




More information about the svn-commits mailing list