[asterisk-bugs] [JIRA] (ASTERISK-28950) Stale code in app_queue to check untouched channel

[Walter Doekes (JIRA)]

Walter Doekes created ASTERISK-28950:
----------------------------------------

             Summary: Stale code in app_queue to check untouched channel
                 Key: ASTERISK-28950
                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-28950
             Project: Asterisk
          Issue Type: Bug
      Security Level: None
          Components: Applications/app_queue
    Affects Versions: 13.34.0
            Reporter: Walter Doekes
            Severity: Trivial


Basically, when I fixed up a comment in ASTERISK-28644, I missed that code could've been removed too.

In 465a7518cc, this change was done:
{noformat}
@@ -738,10 +749,7 @@ static int try_calling(struct queue_ent *qe, char *options, char *announceoverri
                }
                /* Special case: If we ring everyone, go ahead and ring them, otherwise
                   just calculate their metric for the appropriate strategy */
-               if (!qe->parent->strategy)
-                       ring_entry(qe, tmp);
-               else
-                       calc_metric(qe->parent, cur, x++, qe, tmp);
+               calc_metric(qe->parent, cur, x++, qe, tmp);
                /* Put them in the list of outgoing thingies...  We're ready now. 
                   XXX If we're forcibly removed, these outgoing calls won't get
                   hung up XXX */
{noformat}
In 161e762742e14, I fixed the stale comment:
{noformat}
@@ -6850,8 +6850,7 @@ static int try_calling(struct queue_ent *qe, struct ast_flags opts, char **opt_a
                tmp->lastcall = cur->lastcall;
                tmp->lastqueue = cur->lastqueue;
                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 */
+               /* Calculate the metric for the appropriate strategy. */
                if (!calc_metric(qe->parent, cur, x++, qe, tmp)) {
                        /* Put them in the list of outgoing thingies...  We're ready now.
                           XXX If we're forcibly removed, these outgoing calls won't get
{noformat}
But I missed the {{if}} after it:
{code}
                /* Calculate the metric for the appropriate strategy. */
                if (!calc_metric(qe->parent, cur, x++, qe, tmp)) {
                        /* Put them in the list of outgoing thingies...  We're ready now.
                           XXX If we're forcibly removed, these outgoing calls won't get
                           hung up XXX */
                        tmp->q_next = outgoing;
                        outgoing = tmp;
                        /* If this line is up, don't try anybody else */
                        if (outgoing->chan && (ast_channel_state(outgoing->chan) == AST_STATE_UP))
                                break;
                } else {
{code}
The {{outgoing->chan}} makes no sense, as {{calc_metric}} does not touch chan. This code should've been removed when {{ring_entry}} was moved.

Suggested fix:
{noformat}
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -6868,9 +6868,6 @@ static int try_calling(struct queue_ent *qe, struct ast_flags opts, char **opt_a
                           hung up XXX */
                        tmp->q_next = outgoing;
                        outgoing = tmp;
-                       /* If this line is up, don't try anybody else */
-                       if (outgoing->chan && (ast_channel_state(outgoing->chan) == AST_STATE_UP))
-                               break;
                } else {
                        callattempt_free(tmp);
                }
{noformat}




--
This message was sent by Atlassian JIRA
(v6.2#6252)