----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3723/#review12804 -----------------------------------------------------------
/team/group/rls/res/res_pjsip_exten_state.c <https://reviewboard.asterisk.org/r/3723/#comment23129> Naming nitpick: typically, allocation routines are named {object}_alloc or something similar: static struct ast_sip_exten_state_data *exten_state_data_alloc(...) /team/group/rls/res/res_pjsip_exten_state.c <https://reviewboard.asterisk.org/r/3723/#comment23130> This function can return -1 on failure. Check for the return and fail allocation if that occurs. /team/group/rls/res/res_pjsip_pubsub.c <https://reviewboard.asterisk.org/r/3723/#comment23132> Given the number of off nominal paths, I'd wrap this up in a RAII_VAR - Matt Jordan On July 8, 2014, 7:47 p.m., Mark Michelson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3723/ > ----------------------------------------------------------- > > (Updated July 8, 2014, 7:47 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-23869 > https://issues.asterisk.org/jira/browse/ASTERISK-23869 > > > Repository: Asterisk > > > Description > ------- > > This adds NOTIFY support for resource list subscriptions. The way this works > is as follows: > > When an initial SUBSCRIBE arrives and the subscription tree is built, all > leaf nodes are called into in order to generate their initial NOTIFY bodies > and store these on their respective subscription nodes. > > Sending a NOTIFY requires traversing the tree. List subscriptions will > generate a multipart/related body with an RLMI part and parts corresponding > to the leaves of the list (at least they will eventually. ASTERISK-23867 > involves doing this part). Single-resource subscriptions read the stored body > on the subscription and uses that to populate the NOTIFY body. > > Leaf nodes in the subscription tree, when they have a state change occur, > call ast_sip_subscription_notify(), as they previously did. > ast_sip_subscription_notify() creates the NOTIFY body for the subscription > and stores that on the subscription in the body_text ast_str field. The > subscription tree is then told to send a NOTIFY if no batching is enabled or > to start a batched NOTIFY if batching is enabled. > > When a resource resubscribes or terminates their subscription, a NOTIFY is > now automatically generated by the pubsub core instead of calling into > subscription handlers. The NOTIFY is built the same way as previously, using > stored NOTIFY bodies on the subscription. This NOTIFY can also cause batched > notification, when the timer fires, not to actually send their batch since it > would be redundant. > > You'll notice the code has been refactored slightly, and a new struct, > sip_subscription_tree, has invaded res_pjsip_pubsub. This is because, as I > was separating the "real" and "virtual" parts of ast_sip_subscriptions out, I > realized that I essentially had two distinct structures. Thus, I separated > the real/meta/base elements of a subscription into the sip_subscription_tree, > and the resource-specific parts into the ast_sip_subscription struct. > sip_subscription_tree is used more heavily in the pubsub core now, whereas > ast_sip_subscription acts as a handle for subscription handlers to use plus a > repository for resource-specific data. > > > Diffs > ----- > > /team/group/rls/res/res_pjsip_pubsub.c 418167 > /team/group/rls/res/res_pjsip_mwi.c 418167 > /team/group/rls/res/res_pjsip_exten_state.c 418167 > /team/group/rls/include/asterisk/res_pjsip_pubsub.h 418167 > > Diff: https://reviewboard.asterisk.org/r/3723/diff/ > > > Testing > ------- > > With this set of changes, I'm still not able to perform RLS-specific tests > since there is still no method of generating multipart/related or RLMI > bodies. However, with these changes, I did run the gamut of subscription > tests in the testsuite and they all pass. This at least means that there are > no detectable regressions at this point. > > > Thanks, > > Mark Michelson > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
