> On July 21, 2014, 1:14 p.m., Mark Michelson wrote: > > /trunk/channels/chan_sip.c, lines 3522-3534 > > <https://reviewboard.asterisk.org/r/3759/diff/3/?file=64859#file64859line3522> > > > > Heh, it's a bit late for this now, but it would have probably saved you > > a bunch of searching and replacing to just change the definition of > > registry_unref() and registry_addref() to use ao2 in their implementations.
I knew this, but decided to put the work in to use the best ao2 function for each use. I do not like the use of intermediary functions or macro's that simply link to ao2 operations. > On July 21, 2014, 1:14 p.m., Mark Michelson wrote: > > /trunk/channels/chan_sip.c, lines 6543-6547 > > <https://reviewboard.asterisk.org/r/3759/diff/3/?file=64859#file64859line6543> > > > > I agree with this comment. Since mwi->call was created via sip_alloc(), > > destruction of it should be accomplished with dialog_unref() I've removed this comment and will post a separate review to fix this in 1.8+. OTOH I think this code is actually unreachable, we have a circular link - mwi->call holds a reference to mwi->call->mwi. I ran testsuite - tests/channels/SIP/subscribe before and after this patch, and mwi leaks. > On July 21, 2014, 1:14 p.m., Mark Michelson wrote: > > /trunk/channels/chan_sip.c, lines 33324-33336 > > <https://reviewboard.asterisk.org/r/3759/diff/3/?file=64859#file64859line33324> > > > > You sometimes search for sip_registrys using OBJ_KEY, but your hash and > > cmp callbacks do not account for a search key being passed to the callback > > instead of a sip_registry. > > > > Use the templates on this wiki page > > https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=25919686 to > > define your hash and comparison callbacks and you should be good to go. I actually only ever used OBJ_KEY. I've fixed the hash/cmp callbacks to follow the template, supporting OBJ_SEARCH_OBJECT in case it's used in the future. I've also replaced the use deprecated OBJ_KEY with OBJ_SEARCH_KEY. When I originally ran the testsuite this was testing for reg1->name, which was a char[80] defined as the first field. Since ®1->name == reg1, it didn't cause a problem. Now that reg1->configvalue is a stringfield, I think this would have caused a segfault, though I didn't get one with the testsuite. - Corey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3759/#review12784 ----------------------------------------------------------- On July 21, 2014, 11:52 p.m., Corey Farrell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3759/ > ----------------------------------------------------------- > > (Updated July 21, 2014, 11:52 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24067 > https://issues.asterisk.org/jira/browse/ASTERISK-24067 > > > Repository: Asterisk > > > Description > ------- > > Upgrade all ASTOBJ objects in chan_sip to ao2. > > > Diffs > ----- > > /trunk/channels/sip/include/sip.h 419127 > /trunk/channels/chan_sip.c 419127 > > Diff: https://reviewboard.asterisk.org/r/3759/diff/ > > > Testing > ------- > > Full testsuite run. > > > Thanks, > > Corey Farrell > >
-- _____________________________________________________________________ -- 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
