[asterisk-commits] russell: branch russell/iax_refcount r80293 - in /team/russell/iax_refcount: ...

[SVN commits to the Asterisk project]

Author: russell
Date: Wed Aug 22 12:20:42 2007
New Revision: 80293

URL: http://svn.digium.com/view/asterisk?view=rev&rev=80293
Log:
Note that you can't hold the object lock when decreasing its reference count.
Also tweak some formatting and comments

Modified:
    team/russell/iax_refcount/include/asterisk/astobj2.h
    team/russell/iax_refcount/main/astobj2.c

Modified: team/russell/iax_refcount/include/asterisk/astobj2.h
URL: http://svn.digium.com/view/asterisk/team/russell/iax_refcount/include/asterisk/astobj2.h?view=diff&rev=80293&r1=80292&r2=80293
==============================================================================
--- team/russell/iax_refcount/include/asterisk/astobj2.h (original)
+++ team/russell/iax_refcount/include/asterisk/astobj2.h Wed Aug 22 12:20:42 2007
@@ -171,8 +171,13 @@
  *
  * Increase/decrease the reference counter according
  * the value of delta.
+ *
  * If the refcount goes to zero, the object is destroyed.
- * \note: if we know the pointer to an object, it is because we
+ *
+ * \note The object must not be locked by the caller of this function, as
+ *       it is invalid to try to unlock it after releasing the reference.
+ *
+ * \note if we know the pointer to an object, it is because we
  * have a reference count to it, so the only case when the object
  * can go away is when we release our reference, and it is
  * the last one in existence.

Modified: team/russell/iax_refcount/main/astobj2.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax_refcount/main/astobj2.c?view=diff&rev=80293&r1=80292&r2=80293
==============================================================================
--- team/russell/iax_refcount/main/astobj2.c (original)
+++ team/russell/iax_refcount/main/astobj2.c Wed Aug 22 12:20:42 2007
@@ -71,7 +71,8 @@
 #else
 #include <execinfo.h>    /* for backtrace */
 
-void ao2_bt(void) {
+void ao2_bt(void)
+{
     int c, i;
 #define N1	20
     void *addresses[N1];
@@ -99,9 +100,9 @@
 		return NULL;
 	}
 
-	p = (struct astobj2 *)((char *)(user_data) - sizeof(struct astobj2));
+	p = (struct astobj2 *) ( (char *) user_data - sizeof(struct astobj2));
 	if (AO2_MAGIC != (p->priv_data.magic) ) {
-		ast_verbose("----!!!!!--------- bad magic number 0x%x for %p\n", p->priv_data.magic, p);
+		ast_log(LOG_ERROR, "bad magic number 0x%x for %p\n", p->priv_data.magic, p);
 		p = NULL;
 	}
 
@@ -118,6 +119,7 @@
 {
 	struct astobj2 *p = INTERNAL_OBJ(user_data);
 	if (p == NULL)
+	if (!p)
 		return -1;
 	ast_atomic_fetchadd_int(&ao2.total_locked, 1);
 	return ast_mutex_lock( &p->priv_data.lock );
@@ -137,9 +139,6 @@
  */
 int ao2_ref(void *user_data, const int delta)
 {
-#if 0
- 	ast_log(LOG_NOTICE, "Reference counter changed by %i\n", delta);
-#endif
 	int current_value;
 	int ret;
 	struct astobj2 *obj = INTERNAL_OBJ(user_data);
@@ -155,7 +154,7 @@
 	ret = ast_atomic_fetchadd_int( &obj->priv_data.ref_counter, delta );
 	ast_atomic_fetchadd_int(&ao2.total_refs, delta);
 	current_value = ret + delta;
-	// ast_log(LOG_NOTICE, "refcount %d on object %p\n", current_value, user_data);
+	
 	/* this case must never happen */
 	if (current_value < 0) {
 		ast_log(LOG_ERROR, "refcount %d on object %p\n", current_value, user_data);
@@ -163,18 +162,14 @@
 	}
 
 	if (current_value <= 0) { /* last reference, destroy the object */
-		// ast_log(LOG_NOTICE, "refcount %d on object %p, we destroy the object\n", current_value, user_data);
-
 		if (obj->priv_data.destructor_fn != NULL) 
 			obj->priv_data.destructor_fn(user_data);
 
-		/* XXX should check if locked ? */
 		ast_mutex_destroy(&obj->priv_data.lock);
 		ast_atomic_fetchadd_int(&ao2.total_mem, - obj->priv_data.data_size);
 		/* for safety, zero-out the astobj2 header and also the
 		 * first word of the user-data, which we make sure is always
-		 * allocated.
-		 */
+		 * allocated. */
 		bzero(obj, sizeof(struct astobj2 *) + sizeof(void *) );
 		free(obj);
 		ast_atomic_fetchadd_int(&ao2.total_objects, -1);
@@ -192,16 +187,14 @@
 	/* allocation */
 	struct astobj2 *obj;
 
-	// ast_verbose("ao2_alloc %d\n", data_size);
 	if (data_size < sizeof(void *))
 		data_size = sizeof(void *);
 
-	obj = ast_calloc( 1, sizeof(struct astobj2) + data_size );
+	obj = ast_calloc(1, sizeof(*obj) + data_size);
 
 	if (obj == NULL)
 		return NULL;
 
-	/* init */
 	ast_mutex_init(&obj->priv_data.lock);
 	obj->priv_data.magic = AO2_MAGIC;
 	obj->priv_data.data_size = data_size;
@@ -255,9 +248,14 @@
 	struct bucket buckets[0];
 };
  
-/* it is convenient to have a hash function that always returns 0.
+/*!
+ * \brief always zero hash function
+ *
+ * it is convenient to have a hash function that always returns 0.
  * This is basically used when we want to have a container that is
  * a simple linked list.
+ *
+ * \returns 0
  */
 static int hash_zero(const void *user_obj, const int flags)
 {