Dial and queue connected line update macro not always run when expected.
authorRichard Mudgett <rmudgett@digium.com>
Thu, 20 May 2010 19:40:03 +0000 (19:40 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Thu, 20 May 2010 19:40:03 +0000 (19:40 +0000)
The connected line update macro would not get run if the connected line
number string was empty.  The number could be empty if the connected line
update did not update a number but the name.  It should be run if there
was an AST_CONTROL_CONNECTED_LINE frame received for pending dials and
queues.

Renamed and added some more comments for some confusing identifiers
directly connected to the related code.

Also fixed a memory leak in app_queue.

Review: https://reviewboard.asterisk.org/r/669/

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

apps/app_dial.c
apps/app_queue.c

index 1a9ca89..d3cda82 100644 (file)
@@ -551,7 +551,7 @@ enum {
 
 #define DIAL_STILLGOING      (1 << 31)
 #define DIAL_NOFORWARDHTML   ((uint64_t)1 << 32) /* flags are now 64 bits, so keep it up! */
-#define DIAL_NOCONNECTEDLINE ((uint64_t)1 << 33)
+#define DIAL_CALLERID_ABSENT ((uint64_t)1 << 33) /* TRUE if caller id is not available for connected line. */
 #define OPT_CANCEL_ELSEWHERE ((uint64_t)1 << 34)
 #define OPT_PEER_H           ((uint64_t)1 << 35)
 #define OPT_CALLEE_GO_ON     ((uint64_t)1 << 36)
@@ -634,7 +634,10 @@ struct chanlist {
        struct chanlist *next;
        struct ast_channel *chan;
        uint64_t flags;
+       /*! Saved connected party info from an AST_CONTROL_CONNECTED_LINE. */
        struct ast_party_connected_line connected;
+       /*! TRUE if an AST_CONTROL_CONNECTED_LINE update was saved to the connected element. */
+       unsigned int pending_connected_update:1;
 };
 
 static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str *featurecode);
@@ -976,7 +979,7 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in,
                        ast_channel_make_compatible(outgoing->chan, in);
                }
 
-               if (!ast_test_flag64(peerflags, OPT_IGNORE_CONNECTEDLINE) && !ast_test_flag64(outgoing, DIAL_NOCONNECTEDLINE)) {
+               if (!ast_test_flag64(peerflags, OPT_IGNORE_CONNECTEDLINE) && !ast_test_flag64(outgoing, DIAL_CALLERID_ABSENT)) {
                        ast_channel_lock(outgoing->chan);
                        ast_connected_line_copy_from_caller(&connected_caller, &outgoing->chan->cid);
                        ast_channel_unlock(outgoing->chan);
@@ -1036,11 +1039,11 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in,
                                if (!peer) {
                                        ast_verb(3, "%s answered %s\n", c->name, in->name);
                                        if (!single && !ast_test_flag64(peerflags, OPT_IGNORE_CONNECTEDLINE)) {
-                                               if (o->connected.id.number) {
+                                               if (o->pending_connected_update) {
                                                        if (ast_channel_connected_line_macro(c, in, &o->connected, 1, 0)) {
                                                                ast_channel_update_connected_line(in, &o->connected);
                                                        }
-                                               } else if (!ast_test_flag64(o, DIAL_NOCONNECTEDLINE)) {
+                                               } else if (!ast_test_flag64(o, DIAL_CALLERID_ABSENT)) {
                                                        ast_channel_lock(c);
                                                        ast_connected_line_copy_from_caller(&connected_caller, &c->cid);
                                                        ast_channel_unlock(c);
@@ -1098,11 +1101,11 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in,
                                        if (!peer) {
                                                ast_verb(3, "%s answered %s\n", c->name, in->name);
                                                if (!single && !ast_test_flag64(peerflags, OPT_IGNORE_CONNECTEDLINE)) {
-                                                       if (o->connected.id.number) {
+                                                       if (o->pending_connected_update) {
                                                                if (ast_channel_connected_line_macro(c, in, &o->connected, 1, 0)) {
                                                                        ast_channel_update_connected_line(in, &o->connected);
                                                                }
-                                                       } else if (!ast_test_flag64(o, DIAL_NOCONNECTEDLINE)) {
+                                                       } else if (!ast_test_flag64(o, DIAL_CALLERID_ABSENT)) {
                                                                ast_channel_lock(c);
                                                                ast_connected_line_copy_from_caller(&connected_caller, &c->cid);
                                                                ast_channel_unlock(c);
@@ -1219,6 +1222,7 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in,
                                                ast_connected_line_parse_data(f->data.ptr, f->datalen, &connected);
                                                ast_party_connected_line_set(&o->connected, &connected);
                                                ast_party_connected_line_free(&connected);
+                                               o->pending_connected_update = 1;
                                        } else {
                                                if (ast_channel_connected_line_macro(c, in, f, 1, 1)) {
                                                        ast_indicate_data(in, AST_CONTROL_CONNECTED_LINE, f->data.ptr, f->datalen);
@@ -1918,14 +1922,13 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
 
                ast_channel_lock(chan);
                datastore = ast_channel_datastore_find(chan, &dialed_interface_info, NULL);
-               /* If the incoming channel has previously had connected line information
-                * set on it (perhaps through the CONNECTED_LINE dialplan function) then
-                * seed the calllist's connected line information with this previously
-                * acquired info
+               /*
+                * Seed the chanlist's connected line information with previously
+                * acquired connected line info from the incoming channel.  The
+                * previously acquired connected line info could have been set
+                * through the CONNECTED_LINE dialplan function.
                 */
-               if (chan->connected.id.number) {
-                       ast_party_connected_line_copy(&tmp->connected, &chan->connected);
-               }
+               ast_party_connected_line_copy(&tmp->connected, &chan->connected);
                ast_channel_unlock(chan);
 
                if (datastore)
@@ -2034,7 +2037,7 @@ static int dial_exec_full(struct ast_channel *chan, const char *data, struct ast
                        } else if (!ast_strlen_zero(S_OR(chan->macroexten, chan->exten))) {
                                ast_set_callerid(tc, S_OR(chan->macroexten, chan->exten), NULL, NULL);
                        }
-                       ast_set_flag64(tmp, DIAL_NOCONNECTEDLINE);
+                       ast_set_flag64(tmp, DIAL_CALLERID_ABSENT);
                }
 
                if (ast_test_flag64(peerflags, OPT_FORCECLID) && !ast_strlen_zero(opt_args[OPT_ARG_FORCECLID])) {
index 6a544de..366edf5 100644 (file)
@@ -814,8 +814,12 @@ struct callattempt {
        time_t lastcall;
        struct call_queue *lastqueue;
        struct member *member;
-       unsigned int update_connectedline:1;
+       /*! Saved connected party info from an AST_CONTROL_CONNECTED_LINE. */
        struct ast_party_connected_line connected;
+       /*! TRUE if an AST_CONTROL_CONNECTED_LINE update was saved to the connected element. */
+       unsigned int pending_connected_update:1;
+       /*! TRUE if caller id is not available for connected line */
+       unsigned int dial_callerid_absent:1;
 };
 
 
@@ -2613,6 +2617,24 @@ static void leave_queue(struct queue_ent *qe)
        queue_t_unref(q, "Expire copied reference");
 }
 
+/*!
+ * \internal
+ * \brief Destroy the given callattempt structure and free it.
+ * \since 1.8
+ *
+ * \param doomed callattempt structure to destroy.
+ *
+ * \return Nothing
+ */
+static void callattempt_free(struct callattempt *doomed)
+{
+       if (doomed->member) {
+               ao2_ref(doomed->member, -1);
+       }
+       ast_party_connected_line_free(&doomed->connected);
+       ast_free(doomed);
+}
+
 /*! \brief Hang up a list of outgoing calls */
 static void hangupcalls(struct callattempt *outgoing, struct ast_channel *exception, int cancel_answered_elsewhere)
 {
@@ -2628,9 +2650,7 @@ static void hangupcalls(struct callattempt *outgoing, struct ast_channel *except
                }
                oo = outgoing;
                outgoing = outgoing->q_next;
-               if (oo->member)
-                       ao2_ref(oo->member, -1);
-               ast_free(oo);
+               callattempt_free(oo);
        }
 }
 
@@ -2870,7 +2890,7 @@ static int ring_entry(struct queue_ent *qe, struct callattempt *tmp, int *busies
                } else if (!ast_strlen_zero(S_OR(qe->chan->macroexten, qe->chan->exten))) {
                        ast_set_callerid(tmp->chan, S_OR(qe->chan->macroexten, qe->chan->exten), NULL, NULL); 
                }
-               tmp->update_connectedline = 0;
+               tmp->dial_callerid_absent = 1;
        }
 
        ast_party_redirecting_copy(&tmp->chan->redirecting, &qe->chan->redirecting);
@@ -3188,7 +3208,8 @@ static void rna(int rnatime, struct queue_ent *qe, char *interface, char *member
 }
 
 #define AST_MAX_WATCHERS 256
-/*! \brief Wait for a member to answer the call
+/*!
+ * \brief Wait for a member to answer the call
  *
  * \param[in] qe the queue_ent corresponding to the caller in the queue
  * \param[in] outgoing the list of callattempts. Relevant ones will have their chan and stillgoing parameters non-zero
@@ -3197,6 +3218,7 @@ static void rna(int rnatime, struct queue_ent *qe, char *interface, char *member
  * \param[in] prebusies number of busy members calculated prior to calling wait_for_answer
  * \param[in] caller_disconnect if the 'H' option is used when calling Queue(), this is used to detect if the caller pressed * to disconnect the call
  * \param[in] forwardsallowed used to detect if we should allow call forwarding, based on the 'i' option to Queue()
+ * \param[in] update_connectedline Allow connected line and redirecting updates to pass through.
  *
  * \todo eventually all call forward logic should be intergerated into and replaced by ast_call_forward()
  */
@@ -3296,11 +3318,11 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
                                if (!peer) {
                                        ast_verb(3, "%s answered %s\n", ochan_name, inchan_name);
                                        if (update_connectedline) {
-                                               if (o->connected.id.number) {
+                                               if (o->pending_connected_update) {
                                                        if (ast_channel_connected_line_macro(o->chan, in, &o->connected, 1, 0)) {
                                                                ast_channel_update_connected_line(in, &o->connected);
                                                        }
-                                               } else if (o->update_connectedline) {
+                                               } else if (!o->dial_callerid_absent) {
                                                        ast_channel_lock(o->chan);
                                                        ast_connected_line_copy_from_caller(&connected_caller, &o->chan->cid);
                                                        ast_channel_unlock(o->chan);
@@ -3415,11 +3437,11 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
                                                        if (!peer) {
                                                                ast_verb(3, "%s answered %s\n", ochan_name, inchan_name);
                                                                if (update_connectedline) {
-                                                                       if (o->connected.id.number) {
+                                                                       if (o->pending_connected_update) {
                                                                                if (ast_channel_connected_line_macro(o->chan, in, &o->connected, 1, 0)) {
                                                                                        ast_channel_update_connected_line(in, &o->connected);
                                                                                }
-                                                                       } else if (o->update_connectedline) {
+                                                                       } else if (!o->dial_callerid_absent) {
                                                                                ast_channel_lock(o->chan);
                                                                                ast_connected_line_copy_from_caller(&connected_caller, &o->chan->cid);
                                                                                ast_channel_unlock(o->chan);
@@ -3490,6 +3512,7 @@ static struct callattempt *wait_for_answer(struct queue_ent *qe, struct callatte
                                                                ast_connected_line_parse_data(f->data.ptr, f->datalen, &connected);
                                                                ast_party_connected_line_set(&o->connected, &connected);
                                                                ast_party_connected_line_free(&connected);
+                                                               o->pending_connected_update = 1;
                                                        } else {
                                                                if (ast_channel_connected_line_macro(o->chan, in, f, 1, 1)) {
                                                                        ast_indicate_data(in, AST_CONTROL_CONNECTED_LINE, f->data.ptr, f->datalen);
@@ -4200,7 +4223,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                                ao2_iterator_destroy(&memi);
                                if (need_weight)
                                        ao2_unlock(queues);
-                               free(tmp);
+                               callattempt_free(tmp);
                                goto out;
                        }
                        datastore->inheritance = DATASTORE_INHERIT_FOREVER;
@@ -4210,7 +4233,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                                ao2_iterator_destroy(&memi);
                                if (need_weight)
                                        ao2_unlock(queues);
-                               free(tmp);
+                               callattempt_free(tmp);
                                goto out;
                        }
                        datastore->data = dialed_interfaces;
@@ -4232,19 +4255,8 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                }
                AST_LIST_UNLOCK(dialed_interfaces);
 
-               ast_channel_lock(qe->chan);
-               /* If any pre-existing connected line information exists on this
-                * channel, like from the CONNECTED_LINE dialplan function, use this
-                * to seed the connected line information. It may, of course, be updated
-                * during the call
-                */
-               if (qe->chan->connected.id.number) {
-                       ast_party_connected_line_copy(&tmp->connected, &qe->chan->connected);
-               }
-               ast_channel_unlock(qe->chan);
-               
                if (di) {
-                       free(tmp);
+                       callattempt_free(tmp);
                        continue;
                }
 
@@ -4259,7 +4271,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                                ao2_iterator_destroy(&memi);
                                if (need_weight)
                                        ao2_unlock(queues);
-                               free(tmp);
+                               callattempt_free(tmp);
                                goto out;
                        }
                        strcpy(di->interface, cur->interface);
@@ -4269,11 +4281,20 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                        AST_LIST_UNLOCK(dialed_interfaces);
                }
 
+               ast_channel_lock(qe->chan);
+               /*
+                * Seed the callattempt's connected line information with previously
+                * acquired connected line info from the queued channel.  The
+                * previously acquired connected line info could have been set
+                * through the CONNECTED_LINE dialplan function.
+                */
+               ast_party_connected_line_copy(&tmp->connected, &qe->chan->connected);
+               ast_channel_unlock(qe->chan);
+
                tmp->stillgoing = -1;
-               tmp->member = cur;
+               tmp->member = cur;/* Place the reference for cur into callattempt. */
                tmp->lastcall = cur->lastcall;
                tmp->lastqueue = cur->lastqueue;
-               tmp->update_connectedline = 1;
                ast_copy_string(tmp->interface, cur->interface, sizeof(tmp->interface));
                /* Special case: If we ring everyone, go ahead and ring them, otherwise
                   just calculate their metric for the appropriate strategy */
@@ -4287,8 +4308,7 @@ static int try_calling(struct queue_ent *qe, const char *options, char *announce
                        if (outgoing->chan && (outgoing->chan->_state == AST_STATE_UP))
                                break;
                } else {
-                       ao2_ref(cur, -1);
-                       ast_free(tmp);
+                       callattempt_free(tmp);
                }
        }
        ao2_iterator_destroy(&memi);