[Asterisk-code-review] chan sip: Add dialplan function SIP HEADERS (asterisk[master])

Kirill Katsnelson asteriskteam at digium.com
Tue Aug 1 19:18:55 CDT 2017


Kirill Katsnelson has posted comments on this change. ( https://gerrit.asterisk.org/6119 )

Change subject: chan_sip: Add dialplan function SIP_HEADERS
......................................................................


Patch Set 2:

(5 comments)

> A note in the CHANGES file is needed to let people know about the new function.

Done.

https://gerrit.asterisk.org/#/c/6119/2/channels/chan_sip.c
File channels/chan_sip.c:

https://gerrit.asterisk.org/#/c/6119/2/channels/chan_sip.c@400
PS2, Line 400: 			you may	use <literal>${SIP_HEADERS(X-)}</literal> to enumerate optional extended
> Tab in the middle of the line between "may use".
Done


https://gerrit.asterisk.org/#/c/6119/2/channels/chan_sip.c@23092
PS2, Line 23092: 	SCOPED_CHANNELLOCK(chanlock, chan);
               : 
               : 	if (!chan) {
> Using SCOPED_CHANNELLOCK() and then testing if chan is NULL doesn't work.  
Done


https://gerrit.asterisk.org/#/c/6119/2/channels/chan_sip.c@23108
PS2, Line 23108: 	AST_STANDARD_APP_ARGS(args, data);
> This modifies the data string.  The usual code copies to a local string bef
Oh boy, is not that everywhere around, even in the new(er) code, such as chan_pjsip! Fixed here and in SIP_HEADER() implementation too.


https://gerrit.asterisk.org/#/c/6119/2/channels/chan_sip.c@23115
PS2, Line 23115: 		if (ast_begins_with(header, args.pattern)) {
> Could check for '*' to mean match any header for compatibility with the REF
Done


https://gerrit.asterisk.org/#/c/6119/2/channels/chan_sip.c@23119
PS2, Line 23119: 				struct ast_str *token = ast_str_alloca(100);
> Need to pull the ast_str_alloca() out of the loop because it allocates stac
Done



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2076d3893d03a2f82429f393b5b46db6cf68a267
Gerrit-Change-Number: 6119
Gerrit-PatchSet: 2
Gerrit-Owner: Kirill Katsnelson <kkm at smartaction.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kirill Katsnelson <kkm at smartaction.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Wed, 02 Aug 2017 00:18:55 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170801/44693680/attachment.html>


More information about the asterisk-code-review mailing list