> On Jan. 5, 2015, 10:47 p.m., Mark Michelson wrote: > > branches/13/res/res_pjsip/include/res_pjsip_private.h, lines 123-141 > > <https://reviewboard.asterisk.org/r/4311/diff/1/?file=70133#file70133line123> > > > > I unnastand the need for these functions, but I feel like they could be > > named more appropriately. > > > > First, due to a lack of proper namespacing in C, you have to be careful > > defining functions that are called pjsip_*. It is possible that such a > > function could be added to PJSIP itself and cause a conflict with us. Note > > that this applies to other functions besides the ones I've highlighted here. > > > > Second, the names do not do a great job of differentiating between the > > ast_sip_* versions of the functions. This may be hard to describe in a > > brief function name. > > Kevin Harwell wrote: > How does sip_register_service_no_ref sound? I'd like to avoid prefixing > it with "ast_" since that is usually reserved for exported functions and > these are internal. > > if sip_register_blah is too generic then maybe internal_sip_register? > Thoughts/suggestions?
The "internal" version sounds fine to me. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4311/#review14076 ----------------------------------------------------------- On Jan. 2, 2015, 7:46 p.m., Kevin Harwell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4311/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2015, 7:46 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24485 > https://issues.asterisk.org/jira/browse/ASTERISK-24485 > > > Repository: Asterisk > > > Description > ------- > > The res_pjsip module was previously unloadable. With this patch it can now be > unloaded. > > This patch is based off the original patch on the issue by Corey Farrell with > a few modifications. Removed a few changes not required to make the module > unloadable and also fixed a bug that would cause asterisk to crash on > unloading. > > > Diffs > ----- > > branches/13/res/res_pjsip/pjsip_outbound_auth.c 430178 > branches/13/res/res_pjsip/pjsip_options.c 430178 > branches/13/res/res_pjsip/pjsip_global_headers.c 430178 > branches/13/res/res_pjsip/pjsip_distributor.c 430178 > branches/13/res/res_pjsip/pjsip_configuration.c 430178 > branches/13/res/res_pjsip/location.c 430178 > branches/13/res/res_pjsip/include/res_pjsip_private.h 430178 > branches/13/res/res_pjsip/config_transport.c 430178 > branches/13/res/res_pjsip/config_auth.c 430178 > branches/13/res/res_pjsip.c 430178 > branches/13/main/stasis_message_router.c 430178 > > Diff: https://reviewboard.asterisk.org/r/4311/diff/ > > > Testing > ------- > > Made it so res_pjsip was the only pjsip module loaded and then issued an > unload and noted it unloaded successfully (also loaded/unloaded it several > times from the CLI). Also when loaded and with REF_DEBUG enabled issued a > "core stop gracefully" and made sure there were no ref leaks for the module. > > Also tested unloading with other dependent pjsip modules loaded and noted > that the module would not unload (as it should since dependencies are > currently loaded). And then shutdown asterisk and made sure it did not crash > or anything. > > Started asterisk with nominal and off nominal module and pjsip configurations > to make sure things behaved appropriately (no crashes and such) and then > attempted to, or successfully unload the res_pjsip module. Also made sure > Asterisk continued to shutdown without incident. > > > Thanks, > > Kevin Harwell > >
-- _____________________________________________________________________ -- 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
