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

[Asterisk Team (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-28950?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Asterisk Team updated ASTERISK-28950:
-------------------------------------

    Target Release Version/s: 18.0.0

> 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
>            Assignee: Walter Doekes
>            Severity: Trivial
>      Target Release: 13.35.0, 16.12.0, 17.6.0, 18.0.0
>
>
> 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)