[asterisk-commits] mmichelson: branch mmichelson/ao2_containers r140396 - in /team/mmichelson/ao...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Aug 28 12:07:05 CDT 2008


Author: mmichelson
Date: Thu Aug 28 12:07:03 2008
New Revision: 140396

URL: http://svn.digium.com/view/asterisk?view=rev&rev=140396
Log:
I encountered the problem that the container-specific
code could not access fields of the ao2_container
struct and could not use helper functions like
INTERNAL_OBJ and EXTERNAL_OBJ. I wanted to use some
trickery to make those functions available to the
container code but not to any other code in Asterisk.

While such trickery could probably be accomplished
through passing some pointers and documenting some
necessary semantics for the containers, I felt that
this approach is a good compromise.

With this, I am adding the file
include/asterisk/astobj2_private.h
This file contains the complete structure definition
for ao2_container, as well as the INTERNAL_OBJ
and EXTERNAL_OBJ routines. This file should only
be included by files in the astobj2 family.

Next job is to implement the hashtable allocation
function. Once that is done, I can actually start
trying to compile and test to make sure everything
still works as it is supposed to.


Added:
    team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h   (with props)
Modified:
    team/mmichelson/ao2_containers/include/asterisk/astobj2.h
    team/mmichelson/ao2_containers/main/astobj2.c
    team/mmichelson/ao2_containers/main/astobj2_hashtable.c

Modified: team/mmichelson/ao2_containers/include/asterisk/astobj2.h
URL: http://svn.digium.com/view/asterisk/team/mmichelson/ao2_containers/include/asterisk/astobj2.h?view=diff&rev=140396&r1=140395&r2=140396
==============================================================================
--- team/mmichelson/ao2_containers/include/asterisk/astobj2.h (original)
+++ team/mmichelson/ao2_containers/include/asterisk/astobj2.h Thu Aug 28 12:07:03 2008
@@ -445,15 +445,6 @@
 #endif
 int _ao2_ref_debug(void *o, int delta, char *tag, char *file, int line, const char *funcname);
 int _ao2_ref(void *o, int delta);
-
-/* \brief
- * Reference/unreference an internal astobj2 and return the old refcount.
- *
- * This functions exactly like ao2_ref, but must be used
- * by the ao2_container implementations if they wish to
- * increase the reference count on an object.
- */
-int ao2_internal_ref(void *astobj, const int delta);
 
 /*! \brief
  * Lock an object.

Added: team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h
URL: http://svn.digium.com/view/asterisk/team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h?view=auto&rev=140396
==============================================================================
--- team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h (added)
+++ team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h Thu Aug 28 12:07:03 2008
@@ -1,0 +1,63 @@
+
+/*!
+ * A container; stores the hash and callback functions, information on
+ * the size, the hash bucket heads, and a version number, starting at 0
+ * (for a newly created, empty container)
+ * and incremented every time an object is inserted or deleted.
+ * The assumption is that an object is never moved in a container,
+ * but removed and readded with the new number.
+ * The version number is especially useful when implementing iterators.
+ * In fact, we can associate a unique, monotonically increasing number to
+ * each object, which means that, within an iterator, we can store the
+ * version number of the current object, and easily look for the next one,
+ * which is the next one in the list with a higher number.
+ * Since all objects have a version >0, we can use 0 as a marker for
+ * 'we need the first object in the bucket'.
+ *
+ * \todo Linking and unlink objects is typically expensive, as it
+ * involves a malloc() of a small object which is very inefficient.
+ * To optimize this, we allocate larger arrays of bucket_list's
+ * when we run out of them, and then manage our own freelist.
+ * This will be more efficient as we can do the freelist management while
+ * we hold the lock (that we need anyways).
+ */
+struct ao2_container {
+	const struct ao2_container_impl *impl;
+	ao2_callback_fn *cmp_fn;
+	/*! Number of elements in the container */
+	int elements;
+	/*! described above */
+	int version;
+	/*! container data specific to each implementation */
+	void *private;
+};
+
+/*!
+ * \brief convert from a pointer _p to an astobj2 object
+ *
+ * \return the pointer to the user-defined portion.
+ */
+#define EXTERNAL_OBJ(_p)	((_p) == NULL ? NULL : (_p)->user_data)
+
+/*!
+ * \brief convert from a pointer _p to a user-defined object
+ *
+ * \return the pointer to the astobj2 structure
+ */
+inline struct astobj2 *INTERNAL_OBJ(void *user_data)
+{
+	struct astobj2 *p;
+
+	if (!user_data) {
+		ast_log(LOG_ERROR, "user_data is NULL\n");
+		return NULL;
+	}
+
+	p = (struct astobj2 *) ((char *) user_data - sizeof(*p));
+	if (AO2_MAGIC != (p->priv_data.magic) ) {
+		ast_log(LOG_ERROR, "bad magic number 0x%x for %p\n", p->priv_data.magic, p);
+		p = NULL;
+	}
+
+	return p;
+}

Propchange: team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

Propchange: team/mmichelson/ao2_containers/include/asterisk/astobj2_private.h
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: team/mmichelson/ao2_containers/main/astobj2.c
URL: http://svn.digium.com/view/asterisk/team/mmichelson/ao2_containers/main/astobj2.c?view=diff&rev=140396&r1=140395&r2=140396
==============================================================================
--- team/mmichelson/ao2_containers/main/astobj2.c (original)
+++ team/mmichelson/ao2_containers/main/astobj2.c Thu Aug 28 12:07:03 2008
@@ -25,6 +25,7 @@
 #include "asterisk/astobj2.h"
 #include "asterisk/utils.h"
 #include "asterisk/cli.h"
+#include "asterisk/astobj2_private.h"
 #define REF_FILE "/tmp/refs"
 
 /*!
@@ -97,36 +98,6 @@
 }
 #endif
 
-/*!
- * \brief convert from a pointer _p to a user-defined object
- *
- * \return the pointer to the astobj2 structure
- */
-static inline struct astobj2 *INTERNAL_OBJ(void *user_data)
-{
-	struct astobj2 *p;
-
-	if (!user_data) {
-		ast_log(LOG_ERROR, "user_data is NULL\n");
-		return NULL;
-	}
-
-	p = (struct astobj2 *) ((char *) user_data - sizeof(*p));
-	if (AO2_MAGIC != (p->priv_data.magic) ) {
-		ast_log(LOG_ERROR, "bad magic number 0x%x for %p\n", p->priv_data.magic, p);
-		p = NULL;
-	}
-
-	return p;
-}
-
-/*!
- * \brief convert from a pointer _p to an astobj2 object
- *
- * \return the pointer to the user-defined portion.
- */
-#define EXTERNAL_OBJ(_p)	((_p) == NULL ? NULL : (_p)->user_data)
-
 /* the underlying functions common to debug and non-debug versions */
 
 static int __ao2_ref(void *user_data, const int delta);
@@ -217,12 +188,6 @@
 	return __ao2_ref(user_data, delta);
 }
 
-int ao2_internal_ref(void *obj, const int delta)
-{
-	void *user_data = EXTERNAL_OBJ(obj);
-	return ao2_ref(user_data, delta);
-}
-
 int _ao2_ref(void *user_data, const int delta)
 {
 	struct astobj2 *obj = INTERNAL_OBJ(user_data);
@@ -343,39 +308,6 @@
 /* each bucket in the container is a tailq. */
 AST_LIST_HEAD_NOLOCK(bucket, bucket_list);
 
-/*!
- * A container; stores the hash and callback functions, information on
- * the size, the hash bucket heads, and a version number, starting at 0
- * (for a newly created, empty container)
- * and incremented every time an object is inserted or deleted.
- * The assumption is that an object is never moved in a container,
- * but removed and readded with the new number.
- * The version number is especially useful when implementing iterators.
- * In fact, we can associate a unique, monotonically increasing number to
- * each object, which means that, within an iterator, we can store the
- * version number of the current object, and easily look for the next one,
- * which is the next one in the list with a higher number.
- * Since all objects have a version >0, we can use 0 as a marker for
- * 'we need the first object in the bucket'.
- *
- * \todo Linking and unlink objects is typically expensive, as it
- * involves a malloc() of a small object which is very inefficient.
- * To optimize this, we allocate larger arrays of bucket_list's
- * when we run out of them, and then manage our own freelist.
- * This will be more efficient as we can do the freelist management while
- * we hold the lock (that we need anyways).
- */
-struct ao2_container {
-	const struct ao2_container_impl *impl;
-	ao2_callback_fn *cmp_fn;
-	/*! Number of elements in the container */
-	int elements;
-	/*! described above */
-	int version;
-	/*! container data specific to each implementation */
-	void *private;
-};
- 
 /*!
  * \brief always zero hash function
  *
@@ -389,7 +321,6 @@
 {
 	return 0;
 }
-
 
 /*
  * A container is just an object, after all!

Modified: team/mmichelson/ao2_containers/main/astobj2_hashtable.c
URL: http://svn.digium.com/view/asterisk/team/mmichelson/ao2_containers/main/astobj2_hashtable.c?view=diff&rev=140396&r1=140395&r2=140396
==============================================================================
--- team/mmichelson/ao2_containers/main/astobj2_hashtable.c (original)
+++ team/mmichelson/ao2_containers/main/astobj2_hashtable.c Thu Aug 28 12:07:03 2008
@@ -17,6 +17,7 @@
 #include "asterisk.h"
 
 #include "asterisk/astobj2.h"
+#include "asterisk/astobj2_private.h"
 
 static const struct ao2_container_impl hashtable_impl = {
 	.link = hashtable_link,
@@ -54,10 +55,7 @@
 		return NULL;
 	}
 
-	/*XXX this is hashing the INTERNAL_OBJ'd user data.
-	 * This is incorrect. Fix later
-	 */
-	bucket_number = hash_pvt->hash_fn(newobj);
+	bucket_number = hash_pvt->hash_fn(EXTERNAL_OBJ(newobj));
 
 	bucket_number %= hash_pvt->n_buckets;
 	bl->astobj = newobj;
@@ -96,9 +94,6 @@
 		struct bucket_list *cur;
 
 		AST_LIST_TRAVERSE_SAFE_BEGIN(&hash_pvt->buckets[i], cur, entry) {
-			/* XXX Use of EXTERNAL_OBJ will break compilation here since it is not a public
-			 * call. Need to find a way around this
-			 */
 			int match = cb_fn(EXTERNAL_OBJ(cur->astobj), arg, flags) & (CMP_MATCH | CMP_STOP);
 
 			/* we found the object, performing operations according flags */
@@ -111,7 +106,7 @@
 			/* we have a match (CMP_MATCH) here */
 			if (!(flags & OBJ_NODATA)) {	/* if must return the object, record the value */
 				/* it is important to handle this case before the unlink */
-				ao2_internal_ref(ret, 1);
+				ao2_ref(EXTERNAL_OBJ(ret), 1);
 			}
 
 			if (flags & OBJ_UNLINK) {	/* must unlink */
@@ -122,7 +117,7 @@
 				AST_LIST_REMOVE_CURRENT(entry);
 				/* update number of elements and version */
 				ast_atomic_fetchadd_int(&c->elements, -1);
-				ao2_internal_ref(x->astobj, -1);
+				ao2_ref(EXTERNAL_OBJ(x->astobj), -1);
 				free(x);	/* free the link record */
 			}
 




More information about the asterisk-commits mailing list