[asterisk-commits] rmudgett: trunk r399938 - in /trunk: ./ include/asterisk/ main/ tests/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Sep 27 12:11:25 CDT 2013


Author: rmudgett
Date: Fri Sep 27 12:11:22 2013
New Revision: 399938

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=399938
Log:
astobj2: Remove OBJ_CONTINUE support.

OBJ_CONTINUE was a strange feature that came into the world under
suspicious circumstances to support an abuse of the ao2_container by
chan_iax2.  Since chan_iax2 no longer uses OBJ_CONTINUE, it is safe to
remove it.

The simplified code should help performance slightly and make
understanding the code easier.

Review: https://reviewboard.asterisk.org/r/2887/
........

Merged revisions 399937 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/include/asterisk/astobj2.h
    trunk/main/astobj2.c
    trunk/tests/test_astobj2.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/include/asterisk/astobj2.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/astobj2.h?view=diff&rev=399938&r1=399937&r2=399938
==============================================================================
--- trunk/include/asterisk/astobj2.h (original)
+++ trunk/include/asterisk/astobj2.h Fri Sep 27 12:11:22 2013
@@ -896,34 +896,6 @@
 	 */
 	OBJ_MULTIPLE = (1 << 2),
 	/*!
-	 * \brief Continue if a match is not found.
-	 *
-	 * \details
-	 * This flag forces a whole container search.  The
-	 * OBJ_SEARCH_OBJECT, OBJ_SEARCH_KEY, and OBJ_SEARCH_PARTIAL_KEY
-	 * flags just specify where to start the search in the
-	 * container.  If the search is not stopped early then the
-	 * search _continues_ until the search wraps around to the
-	 * starting point.
-	 *
-	 * Normal searches start where the search key specifies to start
-	 * and end when the search key indicates that the object is not
-	 * in the container.
-	 *
-	 * For hash containers, this tells the ao2_callback() core to
-	 * keep searching through the rest of the buckets if a match is
-	 * not found in the starting bucket defined by the hash value on
-	 * the argument.
-	 *
-	 * For rbtree containers, if the search key is not in the
-	 * container, the search will start either at the first item
-	 * before the search key or the first item after the search key.
-	 *
-	 * \note The supplied ao2_callback_fn is called for every node
-	 * in the container from the starting point.
-	 */
-	OBJ_CONTINUE = (1 << 3),
-	/*!
 	 * \brief Assume that the ao2_container is already locked.
 	 *
 	 * \note For ao2_containers that have mutexes, no locking will
@@ -1127,6 +1099,8 @@
  *   OBJ_SEARCH_OBJECT - if set, 'obj', is an object.
  *   OBJ_SEARCH_KEY - if set, 'obj', is a search key item that is not an object.
  *
+ * \note This function must be idempotent.
+ *
  * \return Computed hash value.
  */
 typedef int (ao2_hash_fn)(const void *obj, int flags);
@@ -1140,6 +1114,8 @@
  *   OBJ_SEARCH_OBJECT - if set, 'obj_right', is an object.
  *   OBJ_SEARCH_KEY - if set, 'obj_right', is a search key item that is not an object.
  *   OBJ_SEARCH_PARTIAL_KEY - if set, 'obj_right', is a partial search key item that is not an object.
+ *
+ * \note This function must be idempotent.
  *
  * \retval <0 if obj_left < obj_right
  * \retval =0 if obj_left == obj_right

Modified: trunk/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/astobj2.c?view=diff&rev=399938&r1=399937&r2=399938
==============================================================================
--- trunk/main/astobj2.c (original)
+++ trunk/main/astobj2.c Fri Sep 27 12:11:22 2013
@@ -2249,8 +2249,6 @@
 struct hash_traversal_state {
 	/*! Active sort function in the traversal if not NULL. */
 	ao2_sort_fn *sort_fn;
-	/*! Node returned in the sorted starting hash bucket if OBJ_CONTINUE flag set. (Reffed) */
-	struct hash_bucket_node *first_node;
 	/*! Saved comparison callback arg pointer. */
 	void *arg;
 	/*! Starting hash bucket */
@@ -2261,8 +2259,6 @@
 	enum search_flags flags;
 	/*! TRUE if it is a descending search */
 	unsigned int descending:1;
-	/*! TRUE if the starting bucket needs to be rechecked because of sorting skips. */
-	unsigned int recheck_starting_bucket:1;
 };
 
 struct hash_traversal_state_check {
@@ -2344,12 +2340,6 @@
 		} else {
 			state->bucket_last = bucket_cur;
 		}
-		if (flags & OBJ_CONTINUE) {
-			state->bucket_last = 0;
-			if (state->sort_fn) {
-				state->recheck_starting_bucket = 1;
-			}
-		}
 		state->bucket_start = bucket_cur;
 
 		/* For each bucket */
@@ -2369,14 +2359,7 @@
 					if (cmp > 0) {
 						continue;
 					}
-					if (flags & OBJ_CONTINUE) {
-						/* Remember first node when we wrap around. */
-						__ao2_ref(node, +1);
-						state->first_node = node;
-
-						/* From now on all nodes are matching */
-						state->sort_fn = NULL;
-					} else if (cmp < 0) {
+					if (cmp < 0) {
 						/* No more nodes in this bucket are possible to match. */
 						break;
 					}
@@ -2385,31 +2368,6 @@
 				/* We have the first traversal node */
 				__ao2_ref(node, +1);
 				return node;
-			}
-
-			/* Was this the starting bucket? */
-			if (bucket_cur == state->bucket_start
-				&& (flags & OBJ_CONTINUE)
-				&& (flags & OBJ_SEARCH_MASK) != OBJ_SEARCH_NONE) {
-				/* In case the bucket was empty or none of the nodes matched. */
-				state->sort_fn = NULL;
-			}
-
-			/* Was this the first container bucket? */
-			if (bucket_cur == 0
-				&& (flags & OBJ_CONTINUE)
-				&& (flags & OBJ_SEARCH_MASK) != OBJ_SEARCH_NONE) {
-				/* Move to the end to ensure we check every bucket */
-				bucket_cur = self->n_buckets;
-				state->bucket_last = state->bucket_start + 1;
-				if (state->recheck_starting_bucket) {
-					/*
-					 * We have to recheck the first part of the starting bucket
-					 * because of sorting skips.
-					 */
-					state->recheck_starting_bucket = 0;
-					--state->bucket_last;
-				}
 			}
 		}
 	} else {
@@ -2424,12 +2382,6 @@
 		} else {
 			state->bucket_last = bucket_cur + 1;
 		}
-		if (flags & OBJ_CONTINUE) {
-			state->bucket_last = self->n_buckets;
-			if (state->sort_fn) {
-				state->recheck_starting_bucket = 1;
-			}
-		}
 		state->bucket_start = bucket_cur;
 
 		/* For each bucket */
@@ -2449,14 +2401,7 @@
 					if (cmp < 0) {
 						continue;
 					}
-					if (flags & OBJ_CONTINUE) {
-						/* Remember first node when we wrap around. */
-						__ao2_ref(node, +1);
-						state->first_node = node;
-
-						/* From now on all nodes are matching */
-						state->sort_fn = NULL;
-					} else if (cmp > 0) {
+					if (cmp > 0) {
 						/* No more nodes in this bucket are possible to match. */
 						break;
 					}
@@ -2465,31 +2410,6 @@
 				/* We have the first traversal node */
 				__ao2_ref(node, +1);
 				return node;
-			}
-
-			/* Was this the starting bucket? */
-			if (bucket_cur == state->bucket_start
-				&& (flags & OBJ_CONTINUE)
-				&& (flags & OBJ_SEARCH_MASK) != OBJ_SEARCH_NONE) {
-				/* In case the bucket was empty or none of the nodes matched. */
-				state->sort_fn = NULL;
-			}
-
-			/* Was this the last container bucket? */
-			if (bucket_cur == self->n_buckets - 1
-				&& (flags & OBJ_CONTINUE)
-				&& (flags & OBJ_SEARCH_MASK) != OBJ_SEARCH_NONE) {
-				/* Move to the beginning to ensure we check every bucket */
-				bucket_cur = -1;
-				state->bucket_last = state->bucket_start;
-				if (state->recheck_starting_bucket) {
-					/*
-					 * We have to recheck the first part of the starting bucket
-					 * because of sorting skips.
-					 */
-					state->recheck_starting_bucket = 0;
-					++state->bucket_last;
-				}
 			}
 		}
 	}
@@ -2539,11 +2459,6 @@
 			for (node = AST_DLLIST_LAST(&self->buckets[bucket_cur].list);
 				node;
 				node = AST_DLLIST_PREV(node, links)) {
-				if (node == state->first_node) {
-					/* We have wrapped back to the starting point. */
-					__ao2_ref(prev, -1);
-					return NULL;
-				}
 				if (!node->common.obj) {
 					/* Node is empty */
 					continue;
@@ -2578,23 +2493,6 @@
 
 hash_descending_resume:;
 			}
-
-			/* Was this the first container bucket? */
-			if (bucket_cur == 0
-				&& (flags & OBJ_CONTINUE)
-				&& (flags & OBJ_SEARCH_MASK) != OBJ_SEARCH_NONE) {
-				/* Move to the end to ensure we check every bucket */
-				bucket_cur = self->n_buckets;
-				state->bucket_last = state->bucket_start + 1;
-				if (state->recheck_starting_bucket) {
-					/*
-					 * We have to recheck the first part of the starting bucket
-					 * because of sorting skips.
-					 */
-					state->recheck_starting_bucket = 0;
-					--state->bucket_last;
-				}
-			}
 		}
 	} else {
 		goto hash_ascending_resume;
@@ -2605,11 +2503,6 @@
 			for (node = AST_DLLIST_FIRST(&self->buckets[bucket_cur].list);
 				node;
 				node = AST_DLLIST_NEXT(node, links)) {
-				if (node == state->first_node) {
-					/* We have wrapped back to the starting point. */
-					__ao2_ref(prev, -1);
-					return NULL;
-				}
 				if (!node->common.obj) {
 					/* Node is empty */
 					continue;
@@ -2644,45 +2537,12 @@
 
 hash_ascending_resume:;
 			}
-
-			/* Was this the last container bucket? */
-			if (bucket_cur == self->n_buckets - 1
-				&& (flags & OBJ_CONTINUE)
-				&& (flags & OBJ_SEARCH_MASK) != OBJ_SEARCH_NONE) {
-				/* Move to the beginning to ensure we check every bucket */
-				bucket_cur = -1;
-				state->bucket_last = state->bucket_start;
-				if (state->recheck_starting_bucket) {
-					/*
-					 * We have to recheck the first part of the starting bucket
-					 * because of sorting skips.
-					 */
-					state->recheck_starting_bucket = 0;
-					++state->bucket_last;
-				}
-			}
 		}
 	}
 
 	/* No more nodes in the container left to traverse. */
 	__ao2_ref(prev, -1);
 	return NULL;
-}
-
-/*!
- * \internal
- * \brief Cleanup the hash container traversal state.
- * \since 12.0.0
- *
- * \param state Traversal state to cleanup.
- *
- * \return Nothing
- */
-static void hash_ao2_find_cleanup(struct hash_traversal_state *state)
-{
-	if (state->first_node) {
-		__ao2_ref(state->first_node, -1);
-	}
 }
 
 /*!
@@ -3110,7 +2970,6 @@
 	.insert = (ao2_container_insert_fn) hash_ao2_insert_node,
 	.traverse_first = (ao2_container_find_first_fn) hash_ao2_find_first,
 	.traverse_next = (ao2_container_find_next_fn) hash_ao2_find_next,
-	.traverse_cleanup = (ao2_container_find_cleanup_fn) hash_ao2_find_cleanup,
 	.iterator_next = (ao2_iterator_next_fn) hash_ao2_iterator_next,
 	.destroy = (ao2_container_destroy_fn) hash_ao2_destroy,
 #if defined(AST_DEVMODE)
@@ -4461,8 +4320,6 @@
 struct rbtree_traversal_state {
 	/*! Active sort function in the traversal if not NULL. */
 	ao2_sort_fn *sort_fn;
-	/*! First node returned if OBJ_CONTINUE flag set. (Reffed) */
-	struct rbtree_node *first_node;
 	/*! Saved comparison callback arg pointer. */
 	void *arg;
 	/*! Saved search flags to control traversing the container. */
@@ -4507,31 +4364,9 @@
 		default:
 		case OBJ_ORDER_ASCENDING:
 			node = rb_node_next(node);
-			if (!node) {
-				if (!state->first_node) {
-					break;
-				}
-				/* Wrap around to the beginning. */
-				node = rb_node_most_left(self->root);
-			}
-			if (node == state->first_node) {
-				/* We have wrapped back to the starting point. */
-				node = NULL;
-			}
 			break;
 		case OBJ_ORDER_DESCENDING:
 			node = rb_node_prev(node);
-			if (!node) {
-				if (!state->first_node) {
-					break;
-				}
-				/* Wrap around to the end. */
-				node = rb_node_most_right(self->root);
-			}
-			if (node == state->first_node) {
-				/* We have wrapped back to the starting point. */
-				node = NULL;
-			}
 			break;
 		case OBJ_ORDER_PRE:
 			node = rb_node_pre(node);
@@ -4590,7 +4425,6 @@
  *   OBJ_SEARCH_OBJECT - if set, 'obj_right', is an object.
  *   OBJ_SEARCH_KEY - if set, 'obj_right', is a search key item that is not an object.
  *   OBJ_SEARCH_PARTIAL_KEY - if set, 'obj_right', is a partial search key item that is not an object.
- *   OBJ_CONTINUE - if set, return node right before or right after search key if not a match.
  * \param bias How to bias search direction for duplicates
  *
  * \retval node on success.
@@ -4644,12 +4478,6 @@
 				}
 
 				/* No match found. */
-				if (flags & OBJ_CONTINUE) {
-					next = rb_node_next_full(node);
-					if (!next) {
-						next = rb_node_prev_full(node);
-					}
-				}
 				return next;
 			}
 		} else {
@@ -4700,9 +4528,6 @@
 				}
 
 				/* No match found. */
-				if (flags & OBJ_CONTINUE) {
-					return node;
-				}
 				return NULL;
 			}
 		}
@@ -4867,34 +4692,9 @@
 		break;
 	}
 
-	if (state->sort_fn && (flags & OBJ_CONTINUE)) {
-		/* Remember first node when we wrap around. */
-		__ao2_ref(node, +1);
-		state->first_node = node;
-
-		/* From now on all nodes are matching */
-		state->sort_fn = NULL;
-	}
-
 	/* We have the first traversal node */
 	__ao2_ref(node, +1);
 	return node;
-}
-
-/*!
- * \internal
- * \brief Cleanup the rbtree container traversal state.
- * \since 12.0.0
- *
- * \param state Traversal state to cleanup.
- *
- * \return Nothing
- */
-static void rb_ao2_find_cleanup(struct rbtree_traversal_state *state)
-{
-	if (state->first_node) {
-		__ao2_ref(state->first_node, -1);
-	}
 }
 
 /*!
@@ -5282,7 +5082,6 @@
 	.insert = (ao2_container_insert_fn) rb_ao2_insert_node,
 	.traverse_first = (ao2_container_find_first_fn) rb_ao2_find_first,
 	.traverse_next = (ao2_container_find_next_fn) rb_ao2_find_next,
-	.traverse_cleanup = (ao2_container_find_cleanup_fn) rb_ao2_find_cleanup,
 	.iterator_next = (ao2_iterator_next_fn) rb_ao2_iterator_next,
 	.destroy = (ao2_container_destroy_fn) rb_ao2_destroy,
 #if defined(AST_DEVMODE)

Modified: trunk/tests/test_astobj2.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_astobj2.c?view=diff&rev=399938&r1=399937&r2=399938
==============================================================================
--- trunk/tests/test_astobj2.c (original)
+++ trunk/tests/test_astobj2.c Fri Sep 27 12:11:22 2013
@@ -85,10 +85,6 @@
 
 /*! Partial search key +/- matching range. */
 int partial_key_match_range;
-/*! Special iax2 OBJ_CONTINUE test.  Bucket selected. */
-int special_bucket;
-/*! Special iax2 OBJ_CONTINUE test.  Object number select. */
-int special_match;
 
 static void test_obj_destructor(void *v_obj)
 {
@@ -137,11 +133,6 @@
 			&& cmp_obj->i <= *i + partial_key_match_range) ? CMP_MATCH : 0;
 	} else {
 		struct test_obj *arg_obj = (struct test_obj *) arg;
-
-		if (!arg_obj) {
-			/* Never match on the special iax2 OBJ_CONTINUE test. */
-			return 0;
-		}
 
 		return (cmp_obj->i == arg_obj->i) ? CMP_MATCH : 0;
 	}
@@ -162,14 +153,6 @@
 	} else {
 		const struct test_obj *hash_obj = obj;
 
-		if (!hash_obj) {
-			/*
-			 * Use the special_bucket as the bucket for the special iax2
-			 * OBJ_CONTINUE test.
-			 */
-			return special_bucket;
-		}
-
 		return hash_obj->i;
 	}
 }
@@ -193,14 +176,6 @@
 		return test_left->i - *i;
 	} else {
 		const struct test_obj *test_right = obj_right;
-
-		if (!test_right) {
-			/*
-			 * Compare with special_match in the special iax2 OBJ_CONTINUE
-			 * test.
-			 */
-			return test_left->i - special_match;
-		}
 
 		return test_left->i - test_right->i;
 	}
@@ -453,22 +428,15 @@
 	struct ao2_iterator it;
 	struct ao2_iterator *mult_it;
 	struct test_obj *obj;
-	ao2_callback_fn *cmp_fn;
 	int n_buckets = 0;
 	int increment = 0;
 	int destructor_count = 0;
-	int count;
 	int num;
 	int res = AST_TEST_PASS;
 
 	c_type = test_container2str(type);
 	ast_test_status_update(test, "Test %d, %s containers (%s).\n",
 		tst_num, c_type, use_sort ? "sorted" : "non-sorted");
-
-	/* Need at least 12 objects for the special iax2 OBJ_CONTINUE test. */
-	if (lim < 12) {
-		lim = 12;
-	}
 
 	c1 = NULL;
 	switch (type) {
@@ -480,10 +448,6 @@
 		break;
 	case TEST_CONTAINER_HASH:
 		n_buckets = (ast_random() % ((lim / 4) + 1)) + 1;
-		if (n_buckets < 6) {
-			/* Need at least 6 buckets for the special iax2 OBJ_CONTINUE test. */
-			n_buckets = 6;
-		}
 		c1 = ao2_t_container_alloc_hash(AO2_ALLOC_OPT_LOCK_MUTEX, 0, n_buckets,
 			test_hash_cb, use_sort ? test_sort_cb : NULL, test_cmp_cb, "test");
 		break;
@@ -542,105 +506,6 @@
 
 	/* Testing ao2_find with OBJ_PARTIAL_KEY */
 	res = test_ao2_find_w_OBJ_PARTIAL_KEY(res, c1, lim, test);
-
-	/*
-	 * Testing ao2_find with OBJ_POINTER | OBJ_UNLINK | OBJ_CONTINUE.
-	 * In this test items are unlinked from c1 and placed in c2.  Then
-	 * unlinked from c2 and placed back into c1.
-	 *
-	 * For this module and set of custom hash/cmp functions, an object
-	 * should only be found if the astobj2 default cmp function is used.
-	 * This test is designed to mimic the chan_iax.c call number use case.
-	 *
-	 * Must test the custom cmp_cb case first since it should never
-	 * find and thus unlink anything for this test.
-	 */
-	for (cmp_fn = test_cmp_cb; ; cmp_fn = NULL) {
-		num = lim;
-		for (count = 0; num && count < 100; ++count) {
-			--num;
-
-			/* This special manipulation is needed for sorted hash buckets. */
-			special_bucket = num;
-			switch (count) {
-			case 0:
-				/* Beyond end of bucket list. */
-				special_match = lim;
-				break;
-			case 1:
-				/* At end of bucket list. */
-				special_match = num;
-				break;
-			case 2:
-				/* In between in middle of bucket list. */
-				special_match = num - 1;
-				break;
-			case 3:
-				/* Beginning of bucket list. */
-				special_match = num % n_buckets;
-				break;
-			case 4:
-				/* Before bucket list. */
-				special_match = -1;
-				break;
-			default:
-				/* Empty bucket list. (If possible to empty it.) */
-				special_match = -1;
-				special_bucket = lim - 1;
-				break;
-			}
-
-			/* ao2_find is just a shortcut notation for ao2_callback(). */
-			obj = ao2_callback(c1, OBJ_POINTER | OBJ_UNLINK | OBJ_CONTINUE, cmp_fn, NULL);
-			if (!obj) {
-				if (!cmp_fn) {
-					ast_test_status_update(test,
-						"ao2_find with OBJ_POINTER | OBJ_UNLINK | OBJ_CONTINUE failed with default cmp_cb.\n");
-					res = AST_TEST_FAIL;
-				}
-			} else {
-				if (cmp_fn) {
-					ast_test_status_update(test,
-						"ao2_find with OBJ_POINTER | OBJ_UNLINK | OBJ_CONTINUE failed with custom cmp_cb.\n");
-					res = AST_TEST_FAIL;
-				}
-				ao2_link(c2, obj);
-				ao2_t_ref(obj, -1, "test");
-			}
-		}
-		if (ao2_container_check(c1, 0)) {
-			ast_test_status_update(test, "container integrity check failed\n");
-			res = AST_TEST_FAIL;
-			goto cleanup;
-		}
-		if (ao2_container_check(c2, 0)) {
-			ast_test_status_update(test, "container integrity check failed\n");
-			res = AST_TEST_FAIL;
-			goto cleanup;
-		}
-		it = ao2_iterator_init(c2, 0);
-		while ((obj = ao2_t_iterator_next(&it, "test"))) {
-			ao2_t_unlink(c2, obj, "test");
-			ao2_t_link(c1, obj, "test");
-			ao2_t_ref(obj, -1, "test");
-		}
-		ao2_iterator_destroy(&it);
-		if (ao2_container_check(c1, 0)) {
-			ast_test_status_update(test, "container integrity check failed\n");
-			res = AST_TEST_FAIL;
-			goto cleanup;
-		}
-		if (ao2_container_check(c2, 0)) {
-			ast_test_status_update(test, "container integrity check failed\n");
-			res = AST_TEST_FAIL;
-			goto cleanup;
-		}
-
-		if (!cmp_fn) {
-			/* Completed testing with custom cmp_cb and default cmp_cb */
-			break;
-		}
-	}
 
 	/* Test Callback with no flags. */
 	increment = 0;




More information about the asterisk-commits mailing list