[Asterisk-code-review] res pjsip: Add AMI action 'PJSIPShowAuths' (asterisk[master])

[sungtae kim]

sungtae kim has posted comments on this change. ( https://gerrit.asterisk.org/7808 )

Change subject: res_pjsip: Add AMI action 'PJSIPShowAuths'
......................................................................


Patch Set 2:

(5 comments)

Thank you for kind review. :)

In my understand, it would be better, if the PJSIP response separated Event name for each Action request(PJSIPShowEndpoint, PJSIPShowAuth).

So, the user can recognize it easily what was the request and purpose was.

https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip.c
File res/res_pjsip.c:

https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip.c@2313
PS1, Line 2313: 	<managerEvent language="en_US" name="AuthList">
> This event is redundant with the AuthDetail event.
Yes, agree.

But No EndpointName info in the AuthList event.
It represent configured auth info only.


https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip.c@2530
PS1, Line 2530: 	<manager name="PJSIPShowAuths" language="en_US">
> The AuthList event is the same as the AuthDetail event.  The EndpointName h
Hm.. got it.

But I still thinking, it has to be separated with AuthDetail event. Because it's part of Action: PJSIPShowEndpoint's response message.


https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip.c@2536
PS1, Line 2536: 			<para>Provides a listing of all Auths. For each Auth an <literal>AuthList</literal> event
              : 			is raised that contains relevant attributes and status information.  Once all
> There shouldn't be a line break immediately after the <para> because the CL
Got it. :)


https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip/config_auth.c
File res/res_pjsip/config_auth.c:

https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip/config_auth.c@208
PS1, Line 208: static int format_ami_authlist_handler(void *obj, void *arg, int flags)
> The AuthList event is the same as the AuthDetail event.  The EndpointName h
Looks better. :)

But, I think it would be worth if the Asterisk sends separated event for each purposes.

AuthDetail is part of another action request's response compare than AuthList.


https://gerrit.asterisk.org/#/c/7808/1/res/res_pjsip/config_auth.c@216
PS1, Line 216: 		return CMP_STOP;
> This is an ao2_callback traversal matching function.  This should be CMP_ST
Good. Fixed it. :)



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a88a75b38a2b1dd9d1de6c0307b20a3f584c817
Gerrit-Change-Number: 7808
Gerrit-PatchSet: 2
Gerrit-Owner: sungtae kim <pchero21 at gmail.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: sungtae kim <pchero21 at gmail.com>
Gerrit-Comment-Date: Sun, 07 Jan 2018 15:44:39 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180107/bf67c747/attachment-0001.html>