Dial and queue connected line update macro not always run when expected.

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

diff --git a/apps/app_dial.c b/apps/app_dial.c
index 1a9ca890026bbcfc76e694ec4ec0f5ee1e0e7b26..d3cda82c10219e42d1bb0442221f4475e434bea9 100644 (file)
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -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])) {

diff --git a/apps/app_queue.c b/apps/app_queue.c
index 6a544de8c29a3ede429ee8a99e146e9803068efd..366edf5c277cd50e9a1853df73a0833b67442e37 100644 (file)
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -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);