[asterisk-commits] rmudgett: branch rmudgett/stasis_cache r408446 - /team/rmudgett/stasis_cache/...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Feb 19 15:00:44 CST 2014


Author: rmudgett
Date: Wed Feb 19 15:00:40 2014
New Revision: 408446

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=408446
Log:
stasus_cache.c: Misc optimizations.

* Fixed return in the wrong place in caching_topic_exec() when clearing a
cache entry.

* Added some missing allocation failure checks in cache_put() and
caching_topic_exec().

* Eliminated unnecessary usage of RAII_VAR().

Modified:
    team/rmudgett/stasis_cache/main/stasis_cache.c

Modified: team/rmudgett/stasis_cache/main/stasis_cache.c
URL: http://svnview.digium.com/svn/asterisk/team/rmudgett/stasis_cache/main/stasis_cache.c?view=diff&rev=408446&r1=408445&r2=408446
==============================================================================
--- team/rmudgett/stasis_cache/main/stasis_cache.c (original)
+++ team/rmudgett/stasis_cache/main/stasis_cache.c Wed Feb 19 15:00:40 2014
@@ -57,7 +57,8 @@
 	struct stasis_subscription *sub;
 };
 
-static void stasis_caching_topic_dtor(void *obj) {
+static void stasis_caching_topic_dtor(void *obj)
+{
 	struct stasis_caching_topic *caching_topic = obj;
 
 	/* Caching topics contain subscriptions, and must be manually
@@ -84,26 +85,28 @@
 
 struct stasis_caching_topic *stasis_caching_unsubscribe(struct stasis_caching_topic *caching_topic)
 {
-	if (caching_topic) {
-		RAII_VAR(struct stasis_caching_topic *, hold_ref, NULL,
-			ao2_cleanup);
-
-		/* The subscription may hold the last reference to this caching
-		 * topic, but we want to make sure the unsubscribe finishes
-		 * before kicking of the caching topic's dtor.
+	if (!caching_topic) {
+		return NULL;
+	}
+
+	/*
+	 * The subscription may hold the last reference to this caching
+	 * topic, but we want to make sure the unsubscribe finishes
+	 * before kicking of the caching topic's dtor.
+	 */
+	ao2_ref(caching_topic, +1);
+
+	if (stasis_subscription_is_subscribed(caching_topic->sub)) {
+		/*
+		 * Increment the reference to hold on to it past the
+		 * unsubscribe. Will be cleaned up in dtor.
 		 */
-		ao2_ref(caching_topic, +1);
-		hold_ref = caching_topic;
-
-		if (stasis_subscription_is_subscribed(caching_topic->sub)) {
-			/* Increment the reference to hold on to it past the
-			 * unsubscribe. Will be cleaned up in dtor. */
-			ao2_ref(caching_topic->sub, +1);
-			stasis_unsubscribe(caching_topic->sub);
-		} else {
-			ast_log(LOG_ERROR, "stasis_caching_topic unsubscribed multiple times\n");
-		}
-	}
+		ao2_ref(caching_topic->sub, +1);
+		stasis_unsubscribe(caching_topic->sub);
+	} else {
+		ast_log(LOG_ERROR, "stasis_caching_topic unsubscribed multiple times\n");
+	}
+	ao2_cleanup(caching_topic);
 	return NULL;
 }
 
@@ -123,24 +126,32 @@
 
 struct cache_entry {
 	struct stasis_message_type *type;
-	char *id;
+	const char *id;
 	struct stasis_message *snapshot;
 };
 
 static void cache_entry_dtor(void *obj)
 {
 	struct cache_entry *entry = obj;
+
 	ao2_cleanup(entry->type);
 	entry->type = NULL;
-	ast_free(entry->id);
+	ast_free((char *) entry->id);
 	entry->id = NULL;
 	ao2_cleanup(entry->snapshot);
 	entry->snapshot = NULL;
 }
 
+static void cache_entry_init(struct cache_entry *entry, struct stasis_message_type *type, const char *id, struct stasis_message *snapshot)
+{
+	entry->type = type;
+	entry->id = id;
+	entry->snapshot = snapshot;
+}
+
 static struct cache_entry *cache_entry_create(struct stasis_message_type *type, const char *id, struct stasis_message *snapshot)
 {
-	RAII_VAR(struct cache_entry *, entry, NULL, ao2_cleanup);
+	struct cache_entry *entry;
 
 	ast_assert(type != NULL);
 	ast_assert(id != NULL);
@@ -151,19 +162,12 @@
 		return NULL;
 	}
 
-	entry->id = ast_strdup(id);
+	cache_entry_init(entry, ao2_bump(type), ast_strdup(id), ao2_bump(snapshot));
 	if (!entry->id) {
-		return NULL;
-	}
-
-	ao2_ref(type, +1);
-	entry->type = type;
-	if (snapshot != NULL) {
-		ao2_ref(snapshot, +1);
-		entry->snapshot = snapshot;
-	}
-
-	ao2_ref(entry, +1);
+		ao2_cleanup(entry);
+		return NULL;
+	}
+
 	return entry;
 }
 
@@ -195,40 +199,40 @@
 
 static void cache_dtor(void *obj)
 {
-        struct stasis_cache *cache = obj;
-
-        ao2_cleanup(cache->entries);
-        cache->entries = NULL;
+	struct stasis_cache *cache = obj;
+
+	ao2_cleanup(cache->entries);
+	cache->entries = NULL;
 }
 
 struct stasis_cache *stasis_cache_create(snapshot_get_id id_fn)
 {
-        RAII_VAR(struct stasis_cache *, cache, NULL, ao2_cleanup);
-
-        cache = ao2_alloc_options(sizeof(*cache), cache_dtor,
+	struct stasis_cache *cache;
+
+	cache = ao2_alloc_options(sizeof(*cache), cache_dtor,
 		AO2_ALLOC_OPT_LOCK_NOLOCK);
-        if (!cache) {
-                return NULL;
-        }
-
-        cache->entries = ao2_container_alloc(NUM_CACHE_BUCKETS, cache_entry_hash,
-                cache_entry_cmp);
-        if (!cache->entries) {
-                return NULL;
-        }
-
-        cache->id_fn = id_fn;
-
-        ao2_ref(cache, +1);
-        return cache;
+	if (!cache) {
+		return NULL;
+	}
+
+	cache->entries = ao2_container_alloc(NUM_CACHE_BUCKETS, cache_entry_hash,
+		cache_entry_cmp);
+	if (!cache->entries) {
+		ao2_cleanup(cache);
+		return NULL;
+	}
+
+	cache->id_fn = id_fn;
+
+	return cache;
 }
 
 static struct stasis_message *cache_put(struct stasis_cache *cache,
 	struct stasis_message_type *type, const char *id,
 	struct stasis_message *new_snapshot)
 {
-	RAII_VAR(struct cache_entry *, new_entry, NULL, ao2_cleanup);
-	RAII_VAR(struct cache_entry *, cached_entry, NULL, ao2_cleanup);
+	struct cache_entry *new_entry;
+	struct cache_entry *cached_entry;
 	struct stasis_message *old_snapshot = NULL;
 
 	ast_assert(cache->entries != NULL);
@@ -236,6 +240,9 @@
 		type == stasis_message_type(new_snapshot));
 
 	new_entry = cache_entry_create(type, id, new_snapshot);
+	if (!new_entry) {
+		return NULL;
+	}
 
 	if (new_snapshot == NULL) {
 		/* Remove entry from cache */
@@ -258,32 +265,36 @@
 			/* Insert into the cache */
 			ao2_link_flags(cache->entries, new_entry, OBJ_NOLOCK);
 		}
-
-	}
-
+	}
+
+	ao2_cleanup(new_entry);
+	ao2_cleanup(cached_entry);
 	return old_snapshot;
 }
 
 struct stasis_message *stasis_cache_get(struct stasis_cache *cache, struct stasis_message_type *type, const char *id)
 {
-	RAII_VAR(struct cache_entry *, search_entry, NULL, ao2_cleanup);
-	RAII_VAR(struct cache_entry *, cached_entry, NULL, ao2_cleanup);
+	struct cache_entry search_entry;
+	struct cache_entry *cached_entry;
+	struct stasis_message *snapshot;
 
 	ast_assert(cache->entries != NULL);
-
-	search_entry = cache_entry_create(type, id, NULL);
-	if (search_entry == NULL) {
-		return NULL;
-	}
-
-	cached_entry = ao2_find(cache->entries, search_entry, OBJ_POINTER);
-	if (cached_entry == NULL) {
+	ast_assert(type != NULL);
+	ast_assert(id != NULL);
+
+	cache_entry_init(&search_entry, type, id, NULL);
+
+	cached_entry = ao2_find(cache->entries, &search_entry, OBJ_POINTER);
+	if (!cached_entry) {
 		return NULL;
 	}
 
 	ast_assert(cached_entry->snapshot != NULL);
-	ao2_ref(cached_entry->snapshot, +1);
-	return cached_entry->snapshot;
+	snapshot = cached_entry->snapshot;
+	ao2_ref(snapshot, +1);
+
+	ao2_cleanup(cached_entry);
+	return snapshot;
 }
 
 struct cache_dump_data {
@@ -325,20 +336,13 @@
 
 struct stasis_message *stasis_cache_clear_create(struct stasis_message *id_message)
 {
-	RAII_VAR(struct stasis_message *, msg, NULL, ao2_cleanup);
-
-	msg = stasis_message_create(stasis_cache_clear_type(), id_message);
-	if (!msg) {
-		return NULL;
-	}
-
-	ao2_ref(msg, +1);
-	return msg;
+	return stasis_message_create(stasis_cache_clear_type(), id_message);
 }
 
 static void stasis_cache_update_dtor(void *obj)
 {
 	struct stasis_cache_update *update = obj;
+
 	ao2_cleanup(update->old_snapshot);
 	update->old_snapshot = NULL;
 	ao2_cleanup(update->new_snapshot);
@@ -349,8 +353,8 @@
 
 static struct stasis_message *update_create(struct stasis_message *old_snapshot, struct stasis_message *new_snapshot)
 {
-	RAII_VAR(struct stasis_cache_update *, update, NULL, ao2_cleanup);
-	RAII_VAR(struct stasis_message *, msg, NULL, ao2_cleanup);
+	struct stasis_cache_update *update;
+	struct stasis_message *msg;
 
 	ast_assert(old_snapshot != NULL || new_snapshot != NULL);
 
@@ -376,11 +380,8 @@
 	}
 
 	msg = stasis_message_create(stasis_cache_update_type(), update);
-	if (!msg) {
-		return NULL;
-	}
-
-	ao2_ref(msg, +1);
+
+	ao2_cleanup(update);
 	return msg;
 }
 
@@ -402,8 +403,6 @@
 
 	/* Handle cache clear event */
 	if (stasis_cache_clear_type() == stasis_message_type(message)) {
-		RAII_VAR(struct stasis_message *, old_snapshot, NULL, ao2_cleanup);
-		RAII_VAR(struct stasis_message *, update, NULL, ao2_cleanup);
 		struct stasis_message *clear_msg = stasis_message_data(message);
 		const char *clear_id = caching_topic->cache->id_fn(clear_msg);
 		struct stasis_message_type *clear_type = stasis_message_type(clear_msg);
@@ -411,36 +410,42 @@
 		ast_assert(clear_type != NULL);
 
 		if (clear_id) {
+			struct stasis_message *old_snapshot;
+
 			old_snapshot = cache_put(caching_topic->cache, clear_type, clear_id, NULL);
 			if (old_snapshot) {
+				struct stasis_message *update;
+
 				update = update_create(old_snapshot, NULL);
-				stasis_publish(caching_topic->topic, update);
+				if (update) {
+					stasis_publish(caching_topic->topic, update);
+				}
+				ao2_cleanup(update);
+				ao2_cleanup(old_snapshot);
 				return;
 			}
 
 			ast_log(LOG_ERROR,
 				"Attempting to remove an item from the %s cache that isn't there: %s %s\n",
 				stasis_topic_name(caching_topic->topic), stasis_message_type_name(clear_type), clear_id);
-			return;
 		}
+		return;
 	}
 
 	id = caching_topic->cache->id_fn(message);
-	if (id == NULL) {
-		/* Object isn't cached; discard */
-	} else {
+	if (id) {
 		/* Update the cache */
-		RAII_VAR(struct stasis_message *, old_snapshot, NULL, ao2_cleanup);
-		RAII_VAR(struct stasis_message *, update, NULL, ao2_cleanup);
+		struct stasis_message *old_snapshot;
+		struct stasis_message *update;
 
 		old_snapshot = cache_put(caching_topic->cache, stasis_message_type(message), id, message);
 
 		update = update_create(old_snapshot, message);
-		if (update == NULL) {
-			return;
+		if (update) {
+			stasis_publish(caching_topic->topic, update);
 		}
-
-		stasis_publish(caching_topic->topic, update);
+		ao2_cleanup(update);
+		ao2_cleanup(old_snapshot);
 	}
 }
 




More information about the asterisk-commits mailing list