[Asterisk-code-review] rtp engine: Allow more than 32 dynamic payload types. (asterisk[master])

Corey Farrell asteriskteam at digium.com
Fri Sep 16 17:54:49 CDT 2016


Corey Farrell has posted comments on this change.

Change subject: rtp_engine: Allow more than 32 dynamic payload types.
......................................................................


Patch Set 5: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/3680/5/main/rtp_engine.c
File main/rtp_engine.c:

Line 2704: 	if (0 < cfg) {
> I hate that ast_config_load2 overloads the return values here.  I don't kno
This assumption would also break if we ever decide that ast_config_load2 should return a valid pointer to a 'static struct ast_config' for error's.  Also aren't pointers really unsigned?  (cfg == (void*)-1) might work since it one side would be converted to signed or unsigned.  (0 < cfg) would have no reason to convert anything to signed so I'm not sure it could ever be false.

Second issue, why load this from here?  If the option is going to be in asterisk.conf it should probably be loaded by ast_readconfig (asterisk.c) and exposed via options.h.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7bc96ab764bc30098a178b841cbf7146f9d64964
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list