[Asterisk-code-review] res/res pjsip sip message: added sip message event notifier (asterisk[master])

[sungtae kim]

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

Change subject: res/res_pjsip_sip_message: added sip message event notifier
......................................................................


Patch Set 2:

(4 comments)

updated. :)

https://gerrit.asterisk.org/#/c/10715/1/res/res_pjsip_sip_message.c
File res/res_pjsip_sip_message.c:

https://gerrit.asterisk.org/#/c/10715/1/res/res_pjsip_sip_message.c@22
PS1, Line 22: 	<support_level>extended</support_level>
> This should be extended support level, and disabled by default. […]
Got it. Fixed it. :)


https://gerrit.asterisk.org/#/c/10715/1/res/res_pjsip_sip_message.c@68
PS1, Line 68: static struct ast_sip_session_supplement supp_sip_in_req = {
            : 		.priority = AST_SIP_SUPPLEMENT_PRIORITY_CHANNEL + 1,
            : 		.incoming_request = incoming_request_handler,
            : };
            : 
            : static struct ast_sip_session_supplement supp_sip_in_res = {
            : 		.priority = AST_SIP_SUPPLEMENT_PRIORITY_CHANNEL + 1,
            : 		.incoming_response = incoming_response_handler,
            : };
            : 
            : static struct ast_sip_session_supplement supp_sip_out_req = {
            : 		.priority = AST_SIP_SUPPLEMENT_PRIORITY_CHANNEL + 1,
            : 		.outgoing_request = outgoing_request_handler,
            : };
            : 
            : static struct ast_sip_session_supplement supp_sip_out_res = {
            : 		.priority = AST_SIP_SUPPLEMENT_PRIORITY_CHANNEL + 1,
            : 		.outgoing_response = outgoing_response_handler,
            : };
> These can all be combined into a single supplement.
incoming_request requires different type of return value. So, it's not possible to combine it.

incoming_response and outgoing_request/outgoing_response are different input types are different. So I think it's not good to combine it together, and good to leave it separately.

And the other 2 functions for the outgoing_request and outgoing_response are also, I think it's good to leave it as separately. Because easy to read it.


https://gerrit.asterisk.org/#/c/10715/1/res/res_pjsip_sip_message.c@136
PS1, Line 136: 	}
             : 
             : 	return j_res;
             : }
             : 
             : /*!
             :  * outgoing request sip message handler
             :  * \param session
             :  * \param tdata
             :  *
> The API provides a guarantee that these can't happen. Same applies elsewhere.
Got it. Fixed it! :)


https://gerrit.asterisk.org/#/c/10715/1/res/res_pjsip_sip_message.c@247
PS1, Line 247: 	}
> There is a large penalty for creating a channel snapshot. […]
Agree, fixed it!



-- 
To view, visit https://gerrit.asterisk.org/10715
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dedcc678265ebb74d35e7a2f58bc4b5084bcfe1
Gerrit-Change-Number: 10715
Gerrit-PatchSet: 2
Gerrit-Owner: sungtae kim <pchero21 at gmail.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: sungtae kim <pchero21 at gmail.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 22:17:55 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181127/fa802060/attachment-0001.html>