[asterisk-commits] mmichelson: trunk r156883 - in /trunk: ./ apps/ include/asterisk/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Nov 14 10:53:39 CST 2008


Author: mmichelson
Date: Fri Nov 14 10:53:38 2008
New Revision: 156883

URL: http://svn.digium.com/view/asterisk?view=rev&rev=156883
Log:
Fix some refcounting in app_queue.c and change the
hashing used by app_queue.c to be case-insensitive.
This is accomplished by adding a new case-insensitive
hashing function.

This was necessary to prevent bad refcount errors
(and potential crashes) which would occur due to the
fact that queues were initially read from the config
file in a case-sensitive manner. Then, when a user
issued a CLI command or manager action, we allowed
for case-insensitive input and used that input to 
directly try to find the queue in the hash table. The result
was either that we could not find a queue that was input or
worse, we would end up hashing to a completely bogus value
based on the input.

This commit resolves the problem presented in
issue #13703. However, that issue was reported against
1.6.0. Since this fix introduces a behavior change, I am
electing to not place this same fix in to the 1.6.0 or 1.6.1
branches, and instead will opt for a change which does not
change behavior.


Modified:
    trunk/UPGRADE.txt
    trunk/apps/app_queue.c
    trunk/include/asterisk/strings.h

Modified: trunk/UPGRADE.txt
URL: http://svn.digium.com/view/asterisk/trunk/UPGRADE.txt?view=diff&rev=156883&r1=156882&r2=156883
==============================================================================
--- trunk/UPGRADE.txt (original)
+++ trunk/UPGRADE.txt Fri Nov 14 10:53:38 2008
@@ -291,6 +291,13 @@
   new value has been added to the TRANSFER event that indicates the caller's
   original position in the queue they are being transfered from.
 
+* Prior to Asterisk 1.6.2, queue names were treated in a case-sensitive
+  manner, meaning that queues with names like "sales" and "sALeS" would
+  be seen as unique queues. The parsing logic has changed to use case-
+  insensitive comparisons now when originally hashing based on queue
+  names, meaning that now the two queues mentioned as examples earlier
+  will be seen as having the same name.
+
 iLBC Codec:
 
 * Previously, the Asterisk source code distribution included the iLBC

Modified: trunk/apps/app_queue.c
URL: http://svn.digium.com/view/asterisk/trunk/apps/app_queue.c?view=diff&rev=156883&r1=156882&r2=156883
==============================================================================
--- trunk/apps/app_queue.c (original)
+++ trunk/apps/app_queue.c Fri Nov 14 10:53:38 2008
@@ -67,6 +67,7 @@
 #include <sys/time.h>
 #include <sys/signal.h>
 #include <netinet/in.h>
+#include <ctype.h>
 
 #include "asterisk/lock.h"
 #include "asterisk/file.h"
@@ -850,7 +851,8 @@
 static int queue_hash_cb(const void *obj, const int flags)
 {
 	const struct call_queue *q = obj;
-	return ast_str_hash(q->name);
+
+	return ast_str_case_hash(q->name);
 }
 
 static int queue_cmp_cb(void *obj, void *arg, void *data, int flags)
@@ -1671,7 +1673,6 @@
 			/* Delete if unused (else will be deleted when last caller leaves). */
 			ao2_unlink(queues, q);
 			ao2_unlock(q);
-			queue_unref(q);
 		}
 		return NULL;
 	}
@@ -2192,8 +2193,6 @@
 	if (q->dead) {	
 		/* It's dead and nobody is in it, so kill it */
 		ao2_unlink(queues, q);
-		/* unref the container's reference to the queue */
-		queue_unref(q);
 	}
 	/* unref the explicit ref earlier in the function */
 	queue_unref(q);
@@ -2248,11 +2247,10 @@
 			}
 		}
 		ao2_unlock(q);
+		queue_unref(q);
 		if (found) {
-			queue_unref(q);
 			break;
 		}
-		queue_unref(q);
 	}
 	return found;
 }
@@ -4186,6 +4184,8 @@
 			if (!mem->dynamic) {
 				ao2_ref(mem, -1);
 				ao2_unlock(q);
+				queue_unref(q);
+				ao2_unlock(queues);
 				return RES_NOT_DYNAMIC;
 			}
 			q->membercount--;
@@ -5691,13 +5691,15 @@
 		}
 	}
 
-	queue_iter = ao2_iterator_init(queues, 0);
+	queue_iter = ao2_iterator_init(queues, F_AO2I_DONTLOCK);
+	ao2_lock(queues);
 	while ((q = ao2_iterator_next(&queue_iter))) {
 		float sl;
 
 		ao2_lock(q);
 		if (argc == 3 && strcasecmp(q->name, argv[2])) {
 			ao2_unlock(q);
+			queue_unref(q);
 			continue;
 		}
 		found = 1;
@@ -5768,6 +5770,7 @@
 		}
 		queue_unref(q); /* Unref the iterator's reference */
 	}
+	ao2_unlock(queues);
 	if (!found) {
 		if (argc == 3)
 			ast_str_set(&out, 0, "No such queue: %s.", argv[2]);

Modified: trunk/include/asterisk/strings.h
URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/strings.h?view=diff&rev=156883&r1=156882&r2=156883
==============================================================================
--- trunk/include/asterisk/strings.h (original)
+++ trunk/include/asterisk/strings.h Fri Nov 14 10:53:38 2008
@@ -22,6 +22,8 @@
 
 #ifndef _ASTERISK_STRINGS_H
 #define _ASTERISK_STRINGS_H
+
+#include <ctype.h>
 
 #include "asterisk/inline_api.h"
 #include "asterisk/utils.h"
@@ -738,4 +740,21 @@
 	return abs(hash);
 }
 
+/*!
+ * \brief Compute a hash value on a case-insensitive string
+ *
+ * Uses the same hash algorithm as ast_str_hash, but converts
+ * all characters to lowercase prior to computing a hash. This
+ * allows for easy case-insensitive lookups in a hash table.
+ */
+static force_inline int ast_str_case_hash(const char *str)
+{
+	int hash = 5381;
+
+	while (*str) {
+		hash = hash * 33 ^ tolower(*str++);
+	}
+
+	return abs(hash);
+}
 #endif /* _ASTERISK_STRINGS_H */




More information about the asterisk-commits mailing list