[Asterisk-code-review] res pjsip: Implement additional SIP RFCs for Google Voice tr... (asterisk[master])

Nick French asteriskteam at digium.com
Wed Jul 25 17:10:26 CDT 2018


Nick French has posted comments on this change. ( https://gerrit.asterisk.org/9505 )

Change subject: res_pjsip: Implement additional SIP RFCs for Google Voice trunk compatability
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.asterisk.org/#/c/9505/3/res/res_pjsip_outbound_registration.c
File res/res_pjsip_outbound_registration.c:

https://gerrit.asterisk.org/#/c/9505/3/res/res_pjsip_outbound_registration.c@354
PS3, Line 354: static pj_pool_t *reg_pool;
> The use of this pool in this file is problematic. […]
Agreed. Removed in patchset 4.


https://gerrit.asterisk.org/#/c/9505/3/res/res_pjsip_outbound_registration.c@660
PS3, Line 660: 	data = ast_malloc(sizeof(struct stateless_send_resolver_callback_data));
> This can fail and go kaboom.
Agreed. Fixed in patchset 4.


https://gerrit.asterisk.org/#/c/9505/3/res/res_pjsip_outbound_registration.c@2593
PS3, Line 2593: }
> Any reason this couldn't alter the transport information instead of using the callback? Did you just […]
Yes, setting the transport here does not good as pjsip_regc_send() overrides the transport to use the one set by pjsip_regc_set_transport().

callback mechanism is only to avoid a circular dependency between res_pjsip and res_pjsip_outbound_authentication: res_pjsip_outbound_auth depends on res_pjsip, but then the dialog creation code in res_pjsip needs a way to access outbound's global client state to find registration's transport.


https://gerrit.asterisk.org/#/c/9505/3/third-party/pjproject/patches/0110-oauth.patch
File third-party/pjproject/patches/0110-oauth.patch:

https://gerrit.asterisk.org/#/c/9505/3/third-party/pjproject/patches/0110-oauth.patch@70
PS3, Line 70: +		else //if (pj_stricmp(&c->scheme, &pjsip_DIGEST_STR)==0)
> Should this still be commented out or just removed?
Just commenting that the else case is for DIGEST.



-- 
To view, visit https://gerrit.asterisk.org/9505
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: Id214c2d1c550a41fcf564b7df8f3da7be565bd58
Gerrit-Change-Number: 9505
Gerrit-PatchSet: 3
Gerrit-Owner: Nick French <naf at ou.edu>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Nick French <naf at ou.edu>
Gerrit-Comment-Date: Wed, 25 Jul 2018 22:10:26 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180725/c6d82c04/attachment.html>


More information about the asterisk-code-review mailing list