[Asterisk-code-review] multicast RTP: Add dialing options (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: multicast RTP: Add dialing options
......................................................................


Patch Set 1: Code-Review-1

(9 comments)

https://gerrit.asterisk.org/#/c/2910/1/channels/chan_rtp.c
File channels/chan_rtp.c:

PS1, Line 205: 	fmt = ast_multicast_rtp_options_get_format(mcast_options);
             : 	if (!fmt) {
             : 		fmt = ast_format_cap_get_format(cap, 0);
             : 	}
This leaks fmt because it was setup earlier around line 174.


https://gerrit.asterisk.org/#/c/2910/1/include/asterisk/multicast_rtp.h
File include/asterisk/multicast_rtp.h:

Line 51:  * 
red blob


https://gerrit.asterisk.org/#/c/2910/1/res/res_rtp_multicast.c
File res/res_rtp_multicast.c:

PS1, Line 117: AST_APP_OPTIONS(multicast_rtp_options, BEGIN_OPTIONS
             : 	AST_APP_OPTION_ARG('c', OPT_CODEC, OPT_ARG_CODEC),
             : 	AST_APP_OPTION_ARG('l', OPT_LOOP, OPT_ARG_LOOP),
             : 	AST_APP_OPTION_ARG('t', OPT_TTL, OPT_ARG_TTL),
             : 	AST_APP_OPTION_ARG('i', OPT_IF, OPT_ARG_IF),
             : END_OPTIONS );
doxygen describing the options would be nice


Line 130: 	char buf[0];
Add a doxygen note that the type and options strings are stored here.


Line 148: 	strcpy(pos, type);
/* Safe */


Line 152: 	strcpy(pos, options);
/* Safe */


Line 155: 	ast_app_parse_options(multicast_rtp_options, &mcast_options->opts,
ast_app_parse_options can fail if someone forgets to close a parenthesis.  Need to cleanup and return NULL on failure.


Line 217: 	if (sscanf(ttl_str, "%d", &ttl) < 1) {
We probably still should do %30d on sscanf.


Line 238: 	if (sscanf(loop_str, "%hhu", &loop) < 1) {
%30hhu


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I033b706b533f0aa635c342eb738e0bcefa07e219
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes