[Asterisk-code-review] app dial: Add the "Q" option to set the cause on unanswered... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: app_dial:  Add the "Q" option to set the cause on unanswered channels
......................................................................


Patch Set 4: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/4033/4/apps/app_dial.c
File apps/app_dial.c:

Line 389: 						to send no cause.  See the <filename>causes.h</filename> file for the
cause.h is not going to give you the strings you can use.  The strings are in an array in channel.c.


Line 393: 						<para>Only chan_pjsip supports setting the cause to anything other than ANSWERED_ELSEWHERE.</para>
I don't think this is true at all.  I see in chan_sip, and chan_iax2 among others where the cause is used in the hangup.


Line 541: 			<example title="Dial alice and bob and send NO_ANSWER to bob instead of ANSWERED_ELSEWHRE when alice answers">
Typo still here


Line 823: static void hanguptree(struct dial_head *out_chans, struct ast_channel *exception, int hangupcause)
> const?
Const what?

* You cannot add const to out_chans as this function alters it.

* Adding const to exception isn't going to help anything since it is only used once.  Also const channel pointers tend to be counter productive.

* Making hangup cause const is silly as hangupcause is a pass by value parameter.


Line 2811: 					cause = 0;
Change to assigning -1.  They have provided a bogus cause so we should default to answered elsewhere.


-- 
To view, visit https://gerrit.asterisk.org/4033
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I71742e0919aaa16784c30a2b2e73fbeed7672e47
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Badalian Vyacheslav <v.badalyan at open-bs.ru>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes