Misc minor items found in code.

* Add some reentrancy protection in pbx.c when creating the contexts_table
hash table.

* Fix inverted test in chan_sip.c conditional code.

* Fix uninitialized variable and use of the wrong variable in chan_iax2.c.

* Fix test of return value in app_parkandannounce.c.  Explicitly testing
for -1 is bad if the function does not actually return that value when it
fails.

* Fixup some comments and add some curly braces in features.c.

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

diff --git a/apps/app_parkandannounce.c b/apps/app_parkandannounce.c
index cf08a09a77d19e620faec6549ed8157e7abe4121..b04424ad503d151db208f79ec596068b984a7181 100644 (file)
--- a/apps/app_parkandannounce.c
+++ b/apps/app_parkandannounce.c
@@ -142,8 +142,10 @@ static int parkandannounce_exec(struct ast_channel *chan, const char *data)
        before we are done announcing and the channel is messed with, Kablooeee.  So we use Masq to prevent this.  */
 
        res = ast_masq_park_call(chan, NULL, timeout, &lot);
-       if (res == -1)
-               return res;
+       if (res) {
+               /* Parking failed. */
+               return -1;
+       }
 
        ast_verb(3, "Call Parking Called, lot: %d, timeout: %d, context: %s\n", lot, timeout, args.return_context);
 

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 396fbbe1e54e299e37515fc1a802c6a00aa17c37..d3df28f4d47fc1181836e3b7c5b88983613d3fc1 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -9257,7 +9257,8 @@ static void *iax_park_thread(void *stuff)
        struct ast_channel *chan1, *chan2;
        struct iax_dual *d;
        struct ast_frame *f;
-       int ext;
+       int ext = 0;
+
        d = stuff;
        chan1 = d->chan1;
        chan2 = d->chan2;
@@ -10694,7 +10695,7 @@ static int socket_process(struct iax2_thread *thread)
                                        pbx_builtin_setvar_helper(owner, "BLINDTRANSFER", bridged_chan->name);
                                        pbx_builtin_setvar_helper(bridged_chan, "BLINDTRANSFER", owner->name);
 
-                                       if (ast_parking_ext_valid(ies.called_number, c, iaxs[fr->callno]->context)) {
+                                       if (ast_parking_ext_valid(ies.called_number, owner, iaxs[fr->callno]->context)) {
                                                ast_debug(1, "Parking call '%s'\n", bridged_chan->name);
                                                if (iax_park(bridged_chan, owner, ies.called_number)) {
                                                        ast_log(LOG_WARNING, "Failed to park call '%s'\n",

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index bbd2750c8d0a25898f02b70d54fb3daf87ccfcd2..b839ece03409b36d51e9dfb9c6f6f9255a13ae03 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -20764,7 +20764,7 @@ static void *sip_park_thread(void *stuff)
        
 
 #ifdef WHEN_WE_KNOW_THAT_THE_CLIENT_SUPPORTS_MESSAGE
-       if (!res) {
+       if (res) {
                transmit_message_with_text(transferer->tech_pvt, "Unable to park call.\n");
        } else {
                /* Then tell the transferer what happened */

diff --git a/main/features.c b/main/features.c
index 67a7a6785bf0d41ad06fec72f3c7c30f8965242e..65fe7a13999ec06600f8e15fd5494e38f6a70698 100644 (file)
--- a/main/features.c
+++ b/main/features.c
@@ -649,7 +649,8 @@ static int parkinglot_hash_cb(const void *obj, const int flags)
 
 static int parkinglot_cmp_cb(void *obj, void *arg, int flags)
 {
-       struct ast_parkinglot *parkinglot = obj, *parkinglot2 = arg;
+       struct ast_parkinglot *parkinglot = obj;
+       struct ast_parkinglot *parkinglot2 = arg;
 
        return !strcasecmp(parkinglot->name, parkinglot2->name) ? CMP_MATCH | CMP_STOP : 0;
 }
@@ -3406,14 +3407,17 @@ static void clear_dialed_interfaces(struct ast_channel *chan)
 
 /*!
  * \brief bridge the call and set CDR
- * \param chan,peer,config
- * 
+ *
+ * \param chan The bridge considers this channel the caller.
+ * \param peer The bridge considers this channel the callee.
+ * \param config Configuration for this bridge.
+ *
  * Set start time, check for two channels,check if monitor on
  * check for feature activation, create new CDR
  * \retval res on success.
  * \retval -1 on failure to bridge.
  */
-int ast_bridge_call(struct ast_channel *chan,struct ast_channel *peer,struct ast_bridge_config *config)
+int ast_bridge_call(struct ast_channel *chan, struct ast_channel *peer, struct ast_bridge_config *config)
 {
        /* Copy voice back and forth between the two channels.  Give the peer
           the ability to transfer calls with '#<extension' syntax. */
@@ -4324,8 +4328,8 @@ static void *do_parking_thread(void *ignore)
                struct ao2_iterator iter;
                struct ast_parkinglot *curlot;
                int ms = -1;    /* poll2 timeout, uninitialized */
-               iter = ao2_iterator_init(parkinglots, 0);
 
+               iter = ao2_iterator_init(parkinglots, 0);
                while ((curlot = ao2_iterator_next(&iter))) {
                        manage_parkinglot(curlot, pfds, nfds, &new_pfds, &new_nfds, &ms);
                        ao2_ref(curlot, -1);
@@ -5022,8 +5026,9 @@ static int load_config(void)
                        if ((sscanf(var->value, "%30d", &transferdigittimeout) != 1) || (transferdigittimeout < 1)) {
                                ast_log(LOG_WARNING, "%s is not a valid transferdigittimeout\n", var->value);
                                transferdigittimeout = DEFAULT_TRANSFER_DIGIT_TIMEOUT;
-                       } else
+                       } else {
                                transferdigittimeout = transferdigittimeout * 1000;
+                       }
                } else if (!strcasecmp(var->name, "featuredigittimeout")) {
                        if ((sscanf(var->value, "%30d", &featuredigittimeout) != 1) || (featuredigittimeout < 1)) {
                                ast_log(LOG_WARNING, "%s is not a valid featuredigittimeout\n", var->value);
@@ -5033,14 +5038,16 @@ static int load_config(void)
                        if ((sscanf(var->value, "%30d", &atxfernoanswertimeout) != 1) || (atxfernoanswertimeout < 1)) {
                                ast_log(LOG_WARNING, "%s is not a valid atxfernoanswertimeout\n", var->value);
                                atxfernoanswertimeout = DEFAULT_NOANSWER_TIMEOUT_ATTENDED_TRANSFER;
-                       } else
+                       } else {
                                atxfernoanswertimeout = atxfernoanswertimeout * 1000;
+                       }
                } else if (!strcasecmp(var->name, "atxferloopdelay")) {
                        if ((sscanf(var->value, "%30u", &atxferloopdelay) != 1)) {
                                ast_log(LOG_WARNING, "%s is not a valid atxferloopdelay\n", var->value);
                                atxferloopdelay = DEFAULT_ATXFER_LOOP_DELAY;
-                       } else 
+                       } else {
                                atxferloopdelay *= 1000;
+                       }
                } else if (!strcasecmp(var->name, "atxferdropcall")) {
                        atxferdropcall = ast_true(var->value);
                } else if (!strcasecmp(var->name, "atxfercallbackretries")) {
@@ -5051,12 +5058,13 @@ static int load_config(void)
                } else if (!strcasecmp(var->name, "courtesytone")) {
                        ast_copy_string(courtesytone, var->value, sizeof(courtesytone));
                }  else if (!strcasecmp(var->name, "parkedplay")) {
-                       if (!strcasecmp(var->value, "both"))
+                       if (!strcasecmp(var->value, "both")) {
                                parkedplay = 2;
-                       else if (!strcasecmp(var->value, "parked"))
+                       } else if (!strcasecmp(var->value, "parked")) {
                                parkedplay = 1;
-                       else
+                       } else {
                                parkedplay = 0;
+                       }
                } else if (!strcasecmp(var->name, "xfersound")) {
                        ast_copy_string(xfersound, var->value, sizeof(xfersound));
                } else if (!strcasecmp(var->name, "xferfailsound")) {
@@ -5076,11 +5084,12 @@ static int load_config(void)
 
        unmap_features();
        for (var = ast_variable_browse(cfg, "featuremap"); var; var = var->next) {
-               if (remap_feature(var->name, var->value))
+               if (remap_feature(var->name, var->value)) {
                        ast_log(LOG_NOTICE, "Unknown feature '%s'\n", var->name);
+               }
        }
 
-       /* Map a key combination to an application*/
+       /* Map a key combination to an application */
        ast_unregister_features();
        for (var = ast_variable_browse(cfg, "applicationmap"); var; var = var->next) {
                char *tmp_val = ast_strdupa(var->value);
@@ -5180,23 +5189,27 @@ static int load_config(void)
                /* Is this a parkinglot definition ? */
                if (!strncasecmp(ctg, "parkinglot_", strlen("parkinglot_"))) {
                        ast_debug(2, "Found configuration section %s, assume parking context\n", ctg);
-                       if(!build_parkinglot(ctg, ast_variable_browse(cfg, ctg)))
+                       if (!build_parkinglot(ctg, ast_variable_browse(cfg, ctg))) {
                                ast_log(LOG_ERROR, "Could not build parking lot %s. Configuration error.\n", ctg);
-                       else
+                       } else {
                                ast_debug(1, "Configured parking context %s\n", ctg);
+                       }
                        continue;       
                }
+
                /* No, check if it's a group */
                for (i = 0; i < ARRAY_LEN(categories); i++) {
-                       if (!strcasecmp(categories[i], ctg))
+                       if (!strcasecmp(categories[i], ctg)) {
                                break;
+                       }
                }
-
-               if (i < ARRAY_LEN(categories)) 
+               if (i < ARRAY_LEN(categories)) {
                        continue;
+               }
 
-               if (!(fg = register_group(ctg)))
+               if (!(fg = register_group(ctg))) {
                        continue;
+               }
 
                for (var = ast_variable_browse(cfg, ctg); var; var = var->next) {
                        struct ast_call_feature *feature;
@@ -5937,18 +5950,20 @@ int ast_bridge_timelimit(struct ast_channel *chan, struct ast_bridge_config *con
        } else if ( (delta = config->play_warning - config->timelimit) > 0) {
                int w = config->warning_freq;
 
-               /* If the first warning is requested _after_ the entire call would end,
-                  and no warning frequency is requested, then turn off the warning. If
-                  a warning frequency is requested, reduce the 'first warning' time by
-                  that frequency until it falls within the call's total time limit.
-                  Graphically:
-                                 timelim->|    delta        |<-playwarning
-                       0__________________|_________________|
-                                        | w  |    |    |    |
-
-                  so the number of intervals to cut is 1+(delta-1)/w
-               */
-
+               /*
+                * If the first warning is requested _after_ the entire call
+                * would end, and no warning frequency is requested, then turn
+                * off the warning. If a warning frequency is requested, reduce
+                * the 'first warning' time by that frequency until it falls
+                * within the call's total time limit.
+                *
+                * Graphically:
+                *                timelim->|    delta        |<-playwarning
+                *      0__________________|_________________|
+                *                       | w  |    |    |    |
+                *
+                * so the number of intervals to cut is 1+(delta-1)/w
+                */
                if (w == 0) {
                        config->play_warning = 0;
                } else {
@@ -6205,9 +6220,9 @@ int ast_features_init(void)
        }
 
        res |= ast_devstate_prov_add("Park", metermaidstate);
-#ifdef TEST_FRAMEWORK
+#if defined(TEST_FRAMEWORK)
        res |= AST_TEST_REGISTER(features_test);
-#endif
+#endif /* defined(TEST_FRAMEWORK) */
 
        return res;
 }

diff --git a/main/pbx.c b/main/pbx.c
index dc999f4c5a7f684fe3695c34291750ab69f7e2b8..67d95d38052d395e3f2e9a8af367f95c8984664c 100644 (file)
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -7010,12 +7010,17 @@ struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts,
        int length = sizeof(struct ast_context) + strlen(name) + 1;
 
        if (!contexts_table) {
-               contexts_table = ast_hashtab_create(17,
-                                                                                  ast_hashtab_compare_contexts,
-                                                                                  ast_hashtab_resize_java,
-                                                                                  ast_hashtab_newsize_java,
-                                                                                  ast_hashtab_hash_contexts,
-                                                                                  0);
+               /* Protect creation of contexts_table from reentrancy. */
+               ast_wrlock_contexts();
+               if (!contexts_table) {
+                       contexts_table = ast_hashtab_create(17,
+                               ast_hashtab_compare_contexts,
+                               ast_hashtab_resize_java,
+                               ast_hashtab_newsize_java,
+                               ast_hashtab_hash_contexts,
+                               0);
+               }
+               ast_unlock_contexts();
        }
 
        ast_copy_string(search.name, name, sizeof(search.name));
@@ -7235,7 +7240,7 @@ void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_
 
        begintime = ast_tvnow();
        ast_mutex_lock(&context_merge_lock);/* Serialize ast_merge_contexts_and_delete */
-       ast_rdlock_contexts();
+       ast_wrlock_contexts();
        iter = ast_hashtab_start_traversal(contexts_table);
        while ((tmp = ast_hashtab_next(iter))) {
                context_merge(extcontexts, exttable, tmp, registrar);