[Asterisk-code-review] res pjsip pubsub: Prune subs with reliable transports at sta... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/8082 )

Change subject: res_pjsip_pubsub: Prune subs with reliable transports at startup
......................................................................


Patch Set 2: Code-Review-1

(8 comments)

https://gerrit.asterisk.org/#/c/8082/2/include/asterisk/res_pjsip.h
File include/asterisk/res_pjsip.h:

https://gerrit.asterisk.org/#/c/8082/2/include/asterisk/res_pjsip.h@3063
PS2, Line 3063:  * \param data Data to pass to the matcher. May be NULL and does NOT need to be an ao2 object.
Passing data=NULL unregisters all monitors with the provided callback.


https://gerrit.asterisk.org/#/c/8082/2/include/asterisk/res_pjsip.h@3080
PS2, Line 3080:  * \param data Data to pass to the matcher. May be NULL and does NOT need to be an ao2 object.
Passing data=NULL unregisteres all monitors with the provided callback.


https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip.c
File res/res_pjsip.c:

https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip.c@2897
PS2, Line 2897: int ast_sip_will_uri_survive_restart(pjsip_sip_uri *uri, pjsip_rx_data *rdata)
              : {
              : 	struct ast_sip_endpoint *endpoint = ast_pjsip_rdata_get_endpoint(rdata);
We should be passing in endpoint rather than relying on it being stored in rdata.  This will also eliminate unnecessary ao2_ref/unref's of endpoint.


https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip/pjsip_transport_events.c
File res/res_pjsip/pjsip_transport_events.c:

https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip/pjsip_transport_events.c@212
PS2, Line 212: 	for (idx = AST_VECTOR_SIZE(&monitored->monitors); idx--;) {
If we find a monitor to remove we must not decrement idx.  We need to keep idx the same for another loop.  The simplest fix is to ++idx after removal from the vector.


https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip/pjsip_transport_events.c@216
PS2, Line 216: 		if (notifier->cb == cb_data->cb && (!cb_data->data || cb_data->matches(cb_data->data, notifier->data))) {
Need to break the line for 90 columns.


https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip_outbound_registration.c
File res/res_pjsip_outbound_registration.c:

https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip_outbound_registration.c@881
PS2, Line 881: 	ast_sip_transport_monitor_register(transport, registration_transport_shutdown_cb, monitor);
Restore the line break at 90 and this removes any change to this function.


https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip_pubsub.c
File res/res_pjsip_pubsub.c:

https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip_pubsub.c@130
PS2, Line 130: 				<configOption name="prune_on_restart">
prune_on_boot


https://gerrit.asterisk.org/#/c/8082/2/res/res_pjsip_pubsub.c@3053
PS2, Line 3053: 
stray blank line



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee87cf4eb9b7b2b93d5739a72af52d6ca8fbbe36
Gerrit-Change-Number: 8082
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Mon, 29 Jan 2018 22:38:42 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180129/6430bd5e/attachment-0001.html>