[asterisk-commits] trunk r16532 - in /trunk: ./ include/asterisk/ res/ res/snmp/

asterisk-commits at lists.digium.com asterisk-commits at lists.digium.com
Thu Mar 30 10:10:15 MST 2006


Author: rizzo
Date: Thu Mar 30 11:10:11 2006
New Revision: 16532

URL: http://svn.digium.com/view/asterisk?rev=16532&view=rev
Log:
do not export the tzlock and the list head, and introduce a new method,
ast_walk_indications(), to walk through the list of indications.
The new method returns an unlocked record, which is no different from the
behaviour of other existing methods in indications.c
(i.e. they all need to be fixed, with refcounts or some similar
method).

Note that ast_walk_indications() uses the pointer argument only as a
search key, so its implementation is completely safe.

In turn, this change allows the removal of AST_MUTEX_DEFINE_EXPORTED.


Modified:
    trunk/include/asterisk/indications.h
    trunk/indications.c
    trunk/res/res_indications.c
    trunk/res/snmp/agent.c

Modified: trunk/include/asterisk/indications.h
URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/indications.h?rev=16532&r1=16531&r2=16532&view=diff
==============================================================================
--- trunk/include/asterisk/indications.h (original)
+++ trunk/include/asterisk/indications.h Thu Mar 30 11:10:11 2006
@@ -59,29 +59,34 @@
 };
 
 /* set the default tone country */
-extern int ast_set_indication_country(const char *country);
+int ast_set_indication_country(const char *country);
 
 /* locate tone_zone, given the country. if country == NULL, use the default country */
-extern struct tone_zone *ast_get_indication_zone(const char *country);
+struct tone_zone *ast_get_indication_zone(const char *country);
 /* locate a tone_zone_sound, given the tone_zone. if tone_zone == NULL, use the default tone_zone */
-extern struct tone_zone_sound *ast_get_indication_tone(const struct tone_zone *zone, const char *indication);
+struct tone_zone_sound *ast_get_indication_tone(const struct tone_zone *zone, const char *indication);
 
 /* add a new country, if country exists, it will be replaced. */
-extern int ast_register_indication_country(struct tone_zone *country);
+int ast_register_indication_country(struct tone_zone *country);
 /* remove an existing country and all its indications, country must exist */
-extern int ast_unregister_indication_country(const char *country);
+int ast_unregister_indication_country(const char *country);
 /* add a new indication to a tone_zone. tone_zone must exist. if the indication already
  * exists, it will be replaced. */
-extern int ast_register_indication(struct tone_zone *zone, const char *indication, const char *tonelist);
+int ast_register_indication(struct tone_zone *zone, const char *indication, const char *tonelist);
 /* remove an existing tone_zone's indication. tone_zone must exist */
-extern int ast_unregister_indication(struct tone_zone *zone, const char *indication);
+int ast_unregister_indication(struct tone_zone *zone, const char *indication);
 
 /* Start a tone-list going */
 int ast_playtones_start(struct ast_channel *chan, int vol, const char* tonelist, int interruptible);
 /*! Stop the tones from playing */
 void ast_playtones_stop(struct ast_channel *chan);
 
+/* support for walking through a list of indications */
+struct tone_zone *ast_walk_indications(const struct tone_zone *cur);
+
+#if 0
 extern struct tone_zone *tone_zones;
 extern ast_mutex_t tzlock;
+#endif
 
 #endif /* _ASTERISK_INDICATIONS_H */

Modified: trunk/indications.c
URL: http://svn.digium.com/view/asterisk/trunk/indications.c?rev=16532&r1=16531&r2=16532&view=diff
==============================================================================
--- trunk/indications.c (original)
+++ trunk/indications.c Thu Mar 30 11:10:11 2006
@@ -330,22 +330,28 @@
 
 /*--------------------------------------------*/
 
-struct tone_zone *tone_zones;
+static struct tone_zone *tone_zones;
 static struct tone_zone *current_tonezone;
 
 /* Protect the tone_zones list (highly unlikely that two things would change
  * it at the same time, but still! */
-AST_MUTEX_DEFINE_EXPORTED(tzlock);
-/* XXX note - this is the only instance of AST_MUTEX_DEFINE_EXPORTED()
- * in the entire asterisk code base, and should be replaced by a static one.
- * The mutex is declared exported because it is accessed
- * by other files, namely res/snmp/agent.c and res/res_indications.c.
- * However there are also unprotected accesses to the list, because
- * some of the functions below export pointers to the elements, so
- * the entire mechanism is useless.
- * This needs to be fixed by providing functions to navigate in the
- * list, and refcounts to prevent entries from being destroyed.
- */
+AST_MUTEX_DEFINE_STATIC(tzlock);
+
+struct tone_zone *ast_walk_indications(const struct tone_zone *cur)
+{
+	struct tone_zone *tz;
+
+	if (cur == NULL)
+		return tone_zones;
+	ast_mutex_lock(&tzlock);
+	for (tz = tone_zones; tz; tz = tz->next)
+		if (tz == cur)
+			break;
+	if (tz)
+		tz = tz->next;
+	ast_mutex_unlock(&tzlock);
+	return tz;
+}
 
 /* Set global indication country */
 int ast_set_indication_country(const char *country)

Modified: trunk/res/res_indications.c
URL: http://svn.digium.com/view/asterisk/trunk/res/res_indications.c?rev=16532&r1=16531&r2=16532&view=diff
==============================================================================
--- trunk/res/res_indications.c (original)
+++ trunk/res/res_indications.c Thu Mar 30 11:10:11 2006
@@ -149,26 +149,20 @@
  */
 static int handle_show_indications(int fd, int argc, char *argv[])
 {
-	struct tone_zone *tz;
+	struct tone_zone *tz = NULL;
 	char buf[256];
 	int found_country = 0;
 
-	if (ast_mutex_lock(&tzlock)) {
-		ast_log(LOG_WARNING, "Unable to lock tone_zones list\n");
-		return 0;
-	}
 	if (argc == 2) {
 		/* no arguments, show a list of countries */
 		ast_cli(fd,"Country Alias   Description\n"
 			   "===========================\n");
-		for (tz=tone_zones; tz; tz=tz->next) {
+		while ( (tz = ast_walk_indications(tz) ) )
 			ast_cli(fd,"%-7.7s %-7.7s %s\n", tz->country, tz->alias, tz->description);
-		}
-		ast_mutex_unlock(&tzlock);
 		return 0;
 	}
 	/* there was a request for specific country(ies), lets humor them */
-	for (tz=tone_zones; tz; tz=tz->next) {
+	while ( (tz = ast_walk_indications(tz) ) ) {
 		int i,j;
 		for (i=2; i<argc; i++) {
 			if (strcasecmp(tz->country,argv[i])==0 &&
@@ -183,7 +177,8 @@
 				for (i=0; i<tz->nrringcadence; i++) {
 					j += snprintf(buf+j,sizeof(buf)-j,"%d,",tz->ringcadence[i]);
 				}
-				if (tz->nrringcadence) j--;
+				if (tz->nrringcadence)
+					j--;
 				ast_copy_string(buf+j,"\n",sizeof(buf)-j);
 				ast_cli(fd,buf);
 				for (ts=tz->tones; ts; ts=ts->next)
@@ -194,7 +189,6 @@
 	}
 	if (!found_country)
 		ast_cli(fd,"No countries matched your criteria.\n");
-	ast_mutex_unlock(&tzlock);
 	return -1;
 }
 

Modified: trunk/res/snmp/agent.c
URL: http://svn.digium.com/view/asterisk/trunk/res/snmp/agent.c?rev=16532&r1=16531&r2=16532&view=diff
==============================================================================
--- trunk/res/snmp/agent.c (original)
+++ trunk/res/snmp/agent.c Thu Mar 30 11:10:11 2006
@@ -631,22 +631,16 @@
 								  int exact, size_t *var_len, WriteMethod **write_method)
 {
     static unsigned long long_ret;
-    struct tone_zone *tz;
+    struct tone_zone *tz = NULL;
 
     if (header_generic(vp, name, length, exact, var_len, write_method))
 		return NULL;
 
     switch (vp->magic) {
 	case ASTINDCOUNT:
-		if (ast_mutex_lock(&tzlock)) {
-			ast_log(LOG_WARNING, "Unable to lock tone_zones list\n");
-			snmp_log(LOG_ERR, "Unable to lock tone_zones list in ast_var_indications\n");
-			return NULL;
-		}
 		long_ret = 0;
-		for (tz = tone_zones; tz; tz = tz->next)
+		while ( (tz = ast_walk_indications(tz)) )
 			long_ret++;
-		ast_mutex_unlock(&tzlock);
 
 		return (u_char *)&long_ret;
 	case ASTINDCURRENT:
@@ -667,21 +661,15 @@
 									   int exact, size_t *var_len, WriteMethod **write_method)
 {
     static unsigned long long_ret;
-    struct tone_zone *tz;
+    struct tone_zone *tz = NULL;
     int i;
 
     if (header_simple_table(vp, name, length, exact, var_len, write_method, -1))
 		return NULL;
 
-    if (ast_mutex_lock(&tzlock)) {
-		ast_log(LOG_WARNING, "Unable to lock tone_zones list\n");
-		snmp_log(LOG_ERR, "Unable to lock tone_zones list in ast_var_indications_table\n");
-		return NULL;
-    }
     i = name[*length - 1] - 1;
-    for (tz = tone_zones; tz && i; tz = tz->next)
-		i--;
-    ast_mutex_unlock(&tzlock);
+    while ( (tz = ast_walk_indications(tz)) && i )
+	i--;
     if (tz == NULL)
 		return NULL;
 



More information about the asterisk-commits mailing list