> On July 9, 2014, 3:20 p.m., rmudgett wrote: > > /branches/12/main/framehook.c, lines 248-253 > > <https://reviewboard.asterisk.org/r/3721/diff/1/?file=62415#file62415line248> > > > > Framehook is always freed by framehook_detach(). Framehook is used > > after free.
Hmmmm, sure enough. Well, since both channels are locked during this operation it's probably safe enough to perform the fixup first and then detach the old one. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3721/#review12527 ----------------------------------------------------------- On July 7, 2014, 5:21 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3721/ > ----------------------------------------------------------- > > (Updated July 7, 2014, 5:21 p.m.) > > > Review request for Asterisk Developers, Joshua Colp, Matt Jordan, Mark > Michelson, and rmudgett. > > > Repository: Asterisk > > > Description > ------- > > It involves masquerades, so you know it's perilous. It involves FAX, so you > know it's maddening. > > The idea is fairly fairly simple at its core. During a masquerade, > audiohooks and framehooks will now be moved automatically (unless explicitly > set not to via a value in the framehook interface). Framehooks are a little > more complicated and some are simply not inheritable while others may require > some kind of fixup (none currently use this. The framehooks in res_fax ended > up getting a little too wierd to work well with the framehook fixup, so its > fixup had to be handled as part of the datastore fixup function.) > > It might be more appropriate to do this as an Asterisk 13 patch. I'm > uncertain right now. There are some crashes resolved by this (including > https://reviewboard.asterisk.org/r/3603/ which Corey Farrell is currently > trying to solve for 11 and maybe for earlier versions as well), but it's a > moderately big change and it likely has some potential to break things. > > > Diffs > ----- > > /branches/12/res/res_pjsip_refer.c 417692 > /branches/12/res/res_fax.c 417692 > /branches/12/main/framehook.c 417692 > /branches/12/main/channel.c 417692 > /branches/12/main/bridge_basic.c 417692 > /branches/12/main/audiohook.c 417692 > /branches/12/include/asterisk/res_fax.h 417692 > /branches/12/include/asterisk/framehook.h 417692 > /branches/12/include/asterisk/audiohook.h 417692 > /branches/12/funcs/func_audiohookinherit.c 417692 > /branches/12/bridges/bridge_native_rtp.c 417692 > /branches/12/CHANGES 417692 > > Diff: https://reviewboard.asterisk.org/r/3721/diff/ > > > Testing > ------- > > Tested masquerades on channels containing every different type of framehook. > Tested masquerades on channels containing every type of audiohook except for > app_jack (extended support and I haven't every used it, so I would need to > dig into its dependencies... plus none of the audiohooks so far have needed > any special attention anyway and app_jack looks less likely too than some of > the others). > > For everything with obvious audio impact (volume gain, mixmonitors, pitch > shift, chanspy etc), I also checked that audio kept going through as you > would expect it to. I also tested multiple audiohooks and multiple framehooks > for inheritance. Audiohooks were tested for multiple inheritance of the same > type (previously unavailable with the AUDIOHOOK_INHERIT function) > > > Thanks, > > Jonathan Rose > >
-- _____________________________________________________________________ -- 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
