[Asterisk-code-review] res pjsip notify.c: enable in-dialog NOTIFY (asterisk[15])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/8374 )

Change subject: res_pjsip_notify.c: enable in-dialog NOTIFY
......................................................................


Patch Set 2: Code-Review-1

(7 comments)

This change should also be documented in the CHANGES file.

https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c
File res/res_pjsip_notify.c:

https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@468
PS2, Line 468: 	struct notify_channel_data *data = ao2_alloc(sizeof(*data),
This doesn't require a lock so ao2_alloc_options should be used with AO2_ALLOC_OPT_LOCK_NOLOCK


https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@737
PS2, Line 737: 	// keep the channel locked so the dialog can't go away
             : 	ast_debug(1, "Channel has active dialog: %p\n", dlg);
This message isn't useful as it is


https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@742
PS2, Line 742: 		ast_log(LOG_NOTICE, "No active dialog for channel\n");
To a user this is confusing as the channel name is not included


https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@748
PS2, Line 748: 		ast_log(LOG_NOTICE, "Unable to create request to channel");
Same here


https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@757
PS2, Line 757: 		ast_log(LOG_NOTICE, "Unable to send request to channel\n");
Same


https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@848
PS2, Line 848: 	if (!(data = data_create(ch, info))) {
Add a comment that this takes ownership of the channel


https://gerrit.asterisk.org/#/c/8374/2/res/res_pjsip_notify.c@853
PS2, Line 853: 		ao2_cleanup(data);
data can never be NULL here so ao2_ref can be used



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: If7f3151a6d633e414d5dc319d5efc1443c43dd29
Gerrit-Change-Number: 8374
Gerrit-PatchSet: 2
Gerrit-Owner: Nathan Bruning <nathan at iperity.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Mon, 05 Mar 2018 13:16:22 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180305/01649815/attachment.html>