> On Nov. 11, 2014, 6:43 p.m., Mark Michelson wrote: > > If I understand the purpose of p->refer correctly, it's supposed to be > > details relating to a specific REFER (or REFER-esque in some cases) > > transaction. I think that any in-dialog places where p->refer may be > > allocated, the previous p->refer should be freed. In addition to the > > transmit_refer() change you have, this would mean that > > handle_request_refer() and get_also_info() should free p->refer and then > > allocate a new one. > > > > Honestly, the best way to do this is perhaps to just have sip_refer_alloc() > > destroy the old p->refer and then allocate a new one. > > Corey Farrell wrote: > This seams reasonable to me. Now that this will change existing > behaviour, I want to run it through testsuite tests/channels/SIP. Once that > passes I'll update the review. Unless I'm misunderstanding, the call to > sip_refer_alloc from handle_request_invite should destroy any existing > p->refer as well?
It will only happen with INVITEs that start a dialog, so p->refer will be NULL at that point. sip_refer_destroy() will be a no-op, so it's fine. If we get a reinvite with Replaces, Asterisk will send a 400 response before reaching the allocation of p->refer. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4160/#review13722 ----------------------------------------------------------- On Nov. 10, 2014, 6:25 a.m., Corey Farrell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4160/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 6:25 a.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-15242 > https://issues.asterisk.org/jira/browse/ASTERISK-15242 > > > Repository: Asterisk > > > Description > ------- > > If transmit_refer is called when p->refer is already allocated, it will leak > the previous allocation. I checked for all occurrences of sip_refer_alloc, > found that transmit_refer was the only caller that didn't check p->refer > first. This change moves the check for !p->refer to sip_refer_alloc. > > I made transmit_refer destroy any previous p->refer so it will have a clean > structure after reallocation like it does currently. Unsure if it's needed, > but the little bit of extra processing is worth keeping this fix low risk. > > The change is slightly different in 12+, as p->refer->refer_call only exists > in 11. > > > Diffs > ----- > > /branches/11/channels/chan_sip.c 427666 > > Diff: https://reviewboard.asterisk.org/r/4160/diff/ > > > Testing > ------- > > Compiled, visual inspection. > > > 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
