Merged revisions 326985 via svnmerge from
authorRichard Mudgett <rmudgett@digium.com>
Fri, 8 Jul 2011 01:26:01 +0000 (01:26 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Fri, 8 Jul 2011 01:26:01 +0000 (01:26 +0000)
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
  r326985 | rmudgett | 2011-07-07 20:08:05 -0500 (Thu, 07 Jul 2011) | 12 lines

  Some code cleanup in pbx.c

  * Mostly comment and format changes.

  * ast_context_remove_extension_callerid() and ast_add_extension_nolock()
  will write lock the found specific context.

  * ast_context_find() will now tolerate a NULL name.

  * Eliminated some inlined versions of find_context() and
  find_context_locked().
........

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@327000 65c4cc65-6c06-0410-ace0-fbb531ad65f3

main/pbx.c

index 821fdbf..0059cbd 100644 (file)
@@ -12,7 +12,7 @@
  * channels for your use.
  *
  * This program is free software, distributed under the terms of
-* the GNU General Public License Version 2. See the LICENSE file
+ * the GNU General Public License Version 2. See the LICENSE file
  * at the top of the source tree.
  */
 
@@ -921,15 +921,18 @@ struct ast_state_cb {
        int id;
        void *data;
        ast_state_cb_type callback;
+       /*! \note Only used by ast_merge_contexts_and_delete */
        AST_LIST_ENTRY(ast_state_cb) entry;
 };
 
-/*! \brief Structure for dial plan hints
-
-  \note Hints are pointers from an extension in the dialplan to one or
-  more devices (tech/name)
-       - See \ref AstExtState
-*/
+/*!
+ * \brief Structure for dial plan hints
+ *
+ * \note Hints are pointers from an extension in the dialplan to
+ * one or more devices (tech/name)
+ *
+ * See \ref AstExtState
+ */
 struct ast_hint {
        struct ast_exten *exten;        /*!< Extension */
        int laststate;                  /*!< Last known state */
@@ -1114,12 +1117,12 @@ static void __ast_internal_context_destroy( struct ast_context *con);
 static int ast_add_extension_nolock(const char *context, int replace, const char *extension,
        int priority, const char *label, const char *callerid,
        const char *application, void *data, void (*datad)(void *), const char *registrar);
-static int add_pri_lockopt(struct ast_context *con, struct ast_exten *tmp,
-       struct ast_exten *el, struct ast_exten *e, int replace, int lockhints);
 static int ast_add_extension2_lockopt(struct ast_context *con,
        int replace, const char *extension, int priority, const char *label, const char *callerid,
        const char *application, void *data, void (*datad)(void *),
-       const char *registrar, int lockconts, int lockhints);
+       const char *registrar, int lock_context);
+static struct ast_context *find_context_locked(const char *context);
+static struct ast_context *find_context(const char *context);
 
 /* a func for qsort to use to sort a char array */
 static int compare_char(const void *a, const void *b)
@@ -1263,7 +1266,9 @@ static struct pbx_builtin {
 static struct ast_context *contexts;
 static struct ast_hashtab *contexts_table = NULL;
 
-/*!\brief Lock for the ast_context list
+/*!
+ * \brief Lock for the ast_context list
+ * \note
  * This lock MUST be recursive, or a deadlock on reload may result.  See
  * https://issues.asterisk.org/view.php?id=17643
  */
@@ -1274,11 +1279,13 @@ static AST_RWLIST_HEAD_STATIC(apps, ast_app);
 static AST_RWLIST_HEAD_STATIC(switches, ast_switch);
 
 static int stateid = 1;
-/*! 
- * \brief When holding this container's lock, do _not_ do anything that will cause conlock
- * to be taken, unless you _already_ hold it. The ast_merge_contexts_and_delete
- * function will take the locks in conlock/hints order, so any other
- * paths that require both locks must also take them in that order.
+/*!
+ * \note When holding this container's lock, do _not_ do
+ * anything that will cause conlock to be taken, unless you
+ * _already_ hold it.  The ast_merge_contexts_and_delete function
+ * will take the locks in conlock/hints order, so any other
+ * paths that require both locks must also take them in that
+ * order.
  */
 static struct ao2_container *hints;
 
@@ -1306,8 +1313,6 @@ void check_contexts_trouble(void)
        x = 2;
 }
 
-static struct ast_context *find_context_locked(const char *context);
-static struct ast_context *find_context(const char *context);
 int check_contexts(char *, int);
 
 int check_contexts(char *file, int line )
@@ -1353,9 +1358,7 @@ int check_contexts(char *file, int line )
           hashtab structure */
        for(c2=contexts;c2;c2=c2->next) {
                c1 = find_context_locked(c2->name);
-               if (c1)
-               {
-
+               if (c1) {
                        ast_unlock_contexts();
 
                        /* is every entry in the root list also in the root_table? */
@@ -2602,17 +2605,20 @@ struct fake_context /* this struct is purely for matching in the hashtab */
 
 struct ast_context *ast_context_find(const char *name)
 {
-       struct ast_context *tmp = NULL;
+       struct ast_context *tmp;
        struct fake_context item;
 
-       ast_copy_string(item.name, name, sizeof(item.name));
-
+       if (!name) {
+               return NULL;
+       }
        ast_rdlock_contexts();
-       if( contexts_table ) {
-               tmp = ast_hashtab_lookup(contexts_table,&item);
+       if (contexts_table) {
+               ast_copy_string(item.name, name, sizeof(item.name));
+               tmp = ast_hashtab_lookup(contexts_table, &item);
        } else {
-               while ( (tmp = ast_walk_contexts(tmp)) ) {
-                       if (!name || !strcasecmp(name, tmp->name)) {
+               tmp = NULL;
+               while ((tmp = ast_walk_contexts(tmp))) {
+                       if (!strcasecmp(name, tmp->name)) {
                                break;
                        }
                }
@@ -2679,11 +2685,7 @@ struct ast_exten *pbx_find_extension(struct ast_channel *chan,
        if (bypass) { /* bypass means we only look there */
                tmp = bypass;
        } else {      /* look in contexts */
-               struct fake_context item;
-
-               ast_copy_string(item.name, context, sizeof(item.name));
-
-               tmp = ast_hashtab_lookup(contexts_table, &item);
+               tmp = find_context(context);
                if (!tmp) {
                        return NULL;
                }
@@ -4387,9 +4389,9 @@ static int handle_statechange(void *datap)
                        continue;
                }
                hint = device->hint;
+
                /* Get device state for this hint */
                state = ast_extension_state2(hint->exten);
-
                if ((state == -1) || (state == hint->laststate)) {
                        ao2_t_ref(device, -1, "no statechange for device");
                        continue;
@@ -4439,7 +4441,7 @@ static int handle_statechange(void *datap)
 
 /*! \brief  Add watcher for extension states */
 int ast_extension_state_add(const char *context, const char *exten,
-                           ast_state_cb_type callback, void *data)
+       ast_state_cb_type callback, void *data)
 {
        struct ast_hint *hint;
        struct ast_state_cb *state_cb;
@@ -5276,36 +5278,33 @@ void pbx_set_overrideswitch(const char *newval)
 /*!
  * \brief lookup for a context with a given name,
  * \retval found context or NULL if not found.
-*/
+ */
 static struct ast_context *find_context(const char *context)
 {
-       struct ast_context *c = NULL;
        struct fake_context item;
 
        ast_copy_string(item.name, context, sizeof(item.name));
 
-       c = ast_hashtab_lookup(contexts_table,&item);
-
-       return c;
+       return ast_hashtab_lookup(contexts_table, &item);
 }
 
 /*!
  * \brief lookup for a context with a given name,
  * \retval with conlock held if found.
  * \retval NULL if not found.
-*/
+ */
 static struct ast_context *find_context_locked(const char *context)
 {
-       struct ast_context *c = NULL;
+       struct ast_context *c;
        struct fake_context item;
 
        ast_copy_string(item.name, context, sizeof(item.name));
 
        ast_rdlock_contexts();
-       c = ast_hashtab_lookup(contexts_table,&item);
-
-       if (!c)
+       c = ast_hashtab_lookup(contexts_table, &item);
+       if (!c) {
                ast_unlock_contexts();
+       }
 
        return c;
 }
@@ -5315,12 +5314,13 @@ static struct ast_context *find_context_locked(const char *context)
  * This function locks contexts list by &conlist, search for the right context
  * structure, leave context list locked and call ast_context_remove_include2
  * which removes include, unlock contexts list and return ...
-*/
+ */
 int ast_context_remove_include(const char *context, const char *include, const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) {
                /* found, remove include from this context ... */
                ret = ast_context_remove_include2(c, include, registrar);
@@ -5376,8 +5376,9 @@ int ast_context_remove_include2(struct ast_context *con, const char *include, co
 int ast_context_remove_switch(const char *context, const char *sw, const char *data, const char *registrar)
 {
        int ret = -1; /* default error return */
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) {
                /* remove switch from this context ... */
                ret = ast_context_remove_switch2(c, sw, data, registrar);
@@ -5420,11 +5421,7 @@ int ast_context_remove_switch2(struct ast_context *con, const char *sw, const ch
        return ret;
 }
 
-/*
- * \note This functions lock contexts list, search for the right context,
- * call ast_context_remove_extension2, unlock contexts list and return.
- * In this function we are using
- */
+/*! \note This function will lock conlock. */
 int ast_context_remove_extension(const char *context, const char *extension, int priority, const char *registrar)
 {
        return ast_context_remove_extension_callerid(context, extension, priority, NULL, 0, registrar);
@@ -5433,12 +5430,15 @@ int ast_context_remove_extension(const char *context, const char *extension, int
 int ast_context_remove_extension_callerid(const char *context, const char *extension, int priority, const char *callerid, int matchcallerid, const char *registrar)
 {
        int ret = -1; /* default error return */
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) { /* ... remove extension ... */
-               ret = ast_context_remove_extension_callerid2(c, extension, priority, callerid, matchcallerid, registrar, 1);
+               ret = ast_context_remove_extension_callerid2(c, extension, priority, callerid,
+                       matchcallerid, registrar, 0);
                ast_unlock_contexts();
        }
+
        return ret;
 }
 
@@ -5619,21 +5619,14 @@ int ast_context_remove_extension_callerid2(struct ast_context *con, const char *
  */
 int ast_context_lockmacro(const char *context)
 {
-       struct ast_context *c = NULL;
+       struct ast_context *c;
        int ret = -1;
-       struct fake_context item;
-
-       ast_rdlock_contexts();
-
-       ast_copy_string(item.name, context, sizeof(item.name));
 
-       c = ast_hashtab_lookup(contexts_table,&item);
-       if (c)
-               ret = 0;
-       ast_unlock_contexts();
+       c = find_context_locked(context);
+       if (c) {
+               ast_unlock_contexts();
 
-       /* if we found context, lock macrolock */
-       if (ret == 0) {
+               /* if we found context, lock macrolock */
                ret = ast_mutex_lock(&c->macrolock);
        }
 
@@ -5647,21 +5640,14 @@ int ast_context_lockmacro(const char *context)
  */
 int ast_context_unlockmacro(const char *context)
 {
-       struct ast_context *c = NULL;
+       struct ast_context *c;
        int ret = -1;
-       struct fake_context item;
 
-       ast_rdlock_contexts();
-
-       ast_copy_string(item.name, context, sizeof(item.name));
-
-       c = ast_hashtab_lookup(contexts_table,&item);
-       if (c)
-               ret = 0;
-       ast_unlock_contexts();
+       c = find_context_locked(context);
+       if (c) {
+               ast_unlock_contexts();
 
-       /* if we found context, unlock macrolock */
-       if (ret == 0) {
+               /* if we found context, unlock macrolock */
                ret = ast_mutex_unlock(&c->macrolock);
        }
 
@@ -5955,8 +5941,8 @@ static char *handle_show_hints(struct ast_cli_entry *e, int cmd, struct ast_cli_
                        ast_extension_state2str(hint->laststate), watchers);
                num++;
        }
-
        ao2_iterator_destroy(&i);
+
        ast_cli(a->fd, "----------------\n");
        ast_cli(a->fd, "- %d hints registered\n", num);
        return CLI_SUCCESS;
@@ -7214,7 +7200,8 @@ static void context_merge(struct ast_context **extcontexts, struct ast_hashtab *
 void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *registrar)
 {
        double ft;
-       struct ast_context *tmp, *oldcontextslist;
+       struct ast_context *tmp;
+       struct ast_context *oldcontextslist;
        struct ast_hashtab *oldtable;
        struct store_hints store = AST_LIST_HEAD_INIT_VALUE;
        struct store_hint *this;
@@ -7224,17 +7211,22 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_
        struct ast_state_cb *thiscb;
        struct ast_hashtab_iter *iter;
        struct ao2_iterator i;
+       struct timeval begintime;
+       struct timeval writelocktime;
+       struct timeval endlocktime;
+       struct timeval enddeltime;
 
-       /* it is very important that this function hold the hint list lock _and_ the conlock
-          during its operation; not only do we need to ensure that the list of contexts
-          and extensions does not change, but also that no hint callbacks (watchers) are
-          added or removed during the merge/delete process
-
-          in addition, the locks _must_ be taken in this order, because there are already
-          other code paths that use this order
-       */
-
-       struct timeval begintime, writelocktime, endlocktime, enddeltime;
+       /*
+        * It is very important that this function hold the hints
+        * container lock _and_ the conlock during its operation; not
+        * only do we need to ensure that the list of contexts and
+        * extensions does not change, but also that no hint callbacks
+        * (watchers) are added or removed during the merge/delete
+        * process
+        *
+        * In addition, the locks _must_ be taken in this order, because
+        * there are already other code paths that use this order
+        */
 
        begintime = ast_tvnow();
        ast_rdlock_contexts();
@@ -7330,13 +7322,17 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_
        ast_unlock_contexts();
        endlocktime = ast_tvnow();
 
-       /* the old list and hashtab no longer are relevant, delete them while the rest of asterisk
-          is now freely using the new stuff instead */
+       /*
+        * The old list and hashtab no longer are relevant, delete them
+        * while the rest of asterisk is now freely using the new stuff
+        * instead.
+        */
 
        ast_hashtab_destroy(oldtable, NULL);
 
        for (tmp = oldcontextslist; tmp; ) {
                struct ast_context *next;       /* next starting point */
+
                next = tmp->next;
                __ast_internal_context_destroy(tmp);
                tmp = next;
@@ -7358,7 +7354,6 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_
        ft = ast_tvdiff_us(enddeltime, begintime);
        ft /= 1000000.0;
        ast_verb(3,"Total time merge_contexts_delete: %8.6f sec\n", ft);
-       return;
 }
 
 /*
@@ -7369,8 +7364,9 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_
 int ast_context_add_include(const char *context, const char *include, const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) {
                ret = ast_context_add_include2(c, include, registrar);
                ast_unlock_contexts();
@@ -7700,8 +7696,9 @@ int ast_context_add_include2(struct ast_context *con, const char *value,
 int ast_context_add_switch(const char *context, const char *sw, const char *data, int eval, const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) { /* found, add switch to this context */
                ret = ast_context_add_switch2(c, sw, data, eval, registrar);
                ast_unlock_contexts();
@@ -7779,8 +7776,9 @@ int ast_context_add_switch2(struct ast_context *con, const char *value,
 int ast_context_remove_ignorepat(const char *context, const char *ignorepat, const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) {
                ret = ast_context_remove_ignorepat2(c, ignorepat, registrar);
                ast_unlock_contexts();
@@ -7822,8 +7820,9 @@ int ast_context_remove_ignorepat2(struct ast_context *con, const char *ignorepat
 int ast_context_add_ignorepat(const char *context, const char *value, const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) {
                ret = ast_context_add_ignorepat2(c, value, registrar);
                ast_unlock_contexts();
@@ -7872,8 +7871,10 @@ int ast_context_add_ignorepat2(struct ast_context *con, const char *value, const
 int ast_ignore_pattern(const char *context, const char *pattern)
 {
        struct ast_context *con = ast_context_find(context);
+
        if (con) {
                struct ast_ignorepat *pat;
+
                for (pat = con->ignorepats; pat; pat = pat->next) {
                        if (ast_extension_match(pat->pattern, pattern))
                                return 1;
@@ -7893,11 +7894,12 @@ static int ast_add_extension_nolock(const char *context, int replace, const char
        const char *application, void *data, void (*datad)(void *), const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context(context);
+       struct ast_context *c;
 
+       c = find_context(context);
        if (c) {
                ret = ast_add_extension2_lockopt(c, replace, extension, priority, label, callerid,
-                       application, data, datad, registrar, 0, 0);
+                       application, data, datad, registrar, 1);
        }
 
        return ret;
@@ -7912,8 +7914,9 @@ int ast_add_extension(const char *context, int replace, const char *extension,
        const char *application, void *data, void (*datad)(void *), const char *registrar)
 {
        int ret = -1;
-       struct ast_context *c = find_context_locked(context);
+       struct ast_context *c;
 
+       c = find_context_locked(context);
        if (c) {
                ret = ast_add_extension2(c, replace, extension, priority, label, callerid,
                        application, data, datad, registrar);
@@ -8071,20 +8074,9 @@ static int ext_strncpy(char *dst, const char *src, int len)
  * \retval 0 on success.
  * \retval -1 on failure.
 */
-static int add_pri(struct ast_context *con, struct ast_exten *tmp,
+static int add_priority(struct ast_context *con, struct ast_exten *tmp,
        struct ast_exten *el, struct ast_exten *e, int replace)
 {
-       return add_pri_lockopt(con, tmp, el, e, replace, 1);
-}
-
-/*!
- * \brief add the extension in the priority chain.
- * \retval 0 on success.
- * \retval -1 on failure.
-*/
-static int add_pri_lockopt(struct ast_context *con, struct ast_exten *tmp,
-       struct ast_exten *el, struct ast_exten *e, int replace, int lockhints)
-{
        struct ast_exten *ep;
        struct ast_exten *eh=e;
 
@@ -8220,11 +8212,7 @@ static int add_pri_lockopt(struct ast_context *con, struct ast_exten *tmp,
                }
                /* And immediately return success. */
                if (tmp->priority == PRIORITY_HINT) {
-                       if (lockhints) {
-                               ast_add_hint(tmp);
-                       } else {
-                               ast_add_hint(tmp);
-                       }
+                       ast_add_hint(tmp);
                }
        }
        return 0;
@@ -8260,18 +8248,21 @@ int ast_add_extension2(struct ast_context *con,
        const char *application, void *data, void (*datad)(void *),
        const char *registrar)
 {
-       return ast_add_extension2_lockopt(con, replace, extension, priority, label, callerid, application, data, datad, registrar, 1, 1);
+       return ast_add_extension2_lockopt(con, replace, extension, priority, label, callerid,
+               application, data, datad, registrar, 1);
 }
 
-/*! \brief
- * Does all the work of ast_add_extension2, but adds two args, to determine if
- * context and hint locking should be done. In merge_and_delete, we need to do
- * this without locking, as the locks are already held.
+/*!
+ * \brief Same as ast_add_extension2() but controls the context locking.
+ *
+ * \details
+ * Does all the work of ast_add_extension2, but adds an arg to
+ * determine if context locking should be done.
  */
 static int ast_add_extension2_lockopt(struct ast_context *con,
        int replace, const char *extension, int priority, const char *label, const char *callerid,
        const char *application, void *data, void (*datad)(void *),
-       const char *registrar, int lockconts, int lockhints)
+       const char *registrar, int lock_context)
 {
        /*
         * Sort extensions (or patterns) according to the rules indicated above.
@@ -8348,7 +8339,7 @@ static int ast_add_extension2_lockopt(struct ast_context *con,
        tmp->datad = datad;
        tmp->registrar = registrar;
 
-       if (lockconts) {
+       if (lock_context) {
                ast_wrlock_context(con);
        }
 
@@ -8381,9 +8372,9 @@ static int ast_add_extension2_lockopt(struct ast_context *con,
                if (res >= 0)
                        break;
        }
-       if (e && res == 0) { /* exact match, insert in the pri chain */
-               res = add_pri(con, tmp, el, e, replace);
-               if (lockconts) {
+       if (e && res == 0) { /* exact match, insert in the priority chain */
+               res = add_priority(con, tmp, el, e, replace);
+               if (lock_context) {
                        ast_unlock_context(con);
                }
                if (res < 0) {
@@ -8442,15 +8433,11 @@ static int ast_add_extension2_lockopt(struct ast_context *con,
 
                }
                ast_hashtab_insert_safe(con->root_table, tmp);
-               if (lockconts) {
+               if (lock_context) {
                        ast_unlock_context(con);
                }
                if (tmp->priority == PRIORITY_HINT) {
-                       if (lockhints) {
-                               ast_add_hint(tmp);
-                       } else {
-                               ast_add_hint(tmp);
-                       }
+                       ast_add_hint(tmp);
                }
        }
        if (option_debug) {
@@ -10102,17 +10089,17 @@ int load_pbx(void)
 /*
  * Lock context list functions ...
  */
-int ast_wrlock_contexts()
+int ast_wrlock_contexts(void)
 {
        return ast_mutex_lock(&conlock);
 }
 
-int ast_rdlock_contexts()
+int ast_rdlock_contexts(void)
 {
        return ast_mutex_lock(&conlock);
 }
 
-int ast_unlock_contexts()
+int ast_unlock_contexts(void)
 {
        return ast_mutex_unlock(&conlock);
 }