[Asterisk-code-review] res pjsip outbound publish: Add multi-user support per confi... (asterisk[master])

Joshua Colp asteriskteam at digium.com
Wed Apr 27 04:56:19 CDT 2016


Joshua Colp has posted comments on this change.

Change subject: res_pjsip_outbound_publish: Add multi-user support per configuration
......................................................................


Patch Set 1: Code-Review-1

(7 comments)

https://gerrit.asterisk.org/#/c/2709/1/CHANGES
File CHANGES:

Line 240:  to be used for multiple users.
Should be moved over another two spaces.


https://gerrit.asterisk.org/#/c/2709/1/include/asterisk/res_pjsip_outbound_publish.h
File include/asterisk/res_pjsip_outbound_publish.h:

Line 174: int ast_sip_publish_user_send(const char *user, const struct ast_sip_body *body);
Should start with ast_sip_publish_client, not just ast_sip_publish. Goes for the below one too.


PS1, Line 174: int ast_sip_publish_user_send(const char *user, const struct ast_sip_body *body);
             : 
             : /*!
             :  * \brief Remove the given user and stop outbound publishing for it
             :  *
             :  * \param user The user to remove
             :  */
             : void ast_sip_publish_user_remove(const char *user);
This has to still take in a client. Each client has a set of users. Users shouldn't be global.


https://gerrit.asterisk.org/#/c/2709/1/res/res_pjsip_outbound_publish.c
File res/res_pjsip_outbound_publish.c:

Line 200: static struct ao2_container *multi_publishes;
Why did this need to be a global container instead of a container on the publish client itself?


Line 264: static AO2_GLOBAL_OBJ_STATIC(current_states);
I don't think it makes sense anymore to have a global obj container that's swappable if it has to be protected some by a load lock.


Line 824: 	ao2_callback_data(multi_publishes, OBJ_NODATA,
Doesn't this introduce a race condition where if another thread does this at the same time it may create a second user?


Line 987: 	char server_buf[128], to_buf[128], from_buf[128];
This should use the new mechanism for getting the PJSIP configured sizes for these.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib88dde024cc83c916424645d4f5bb84a0fa936cc
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list