----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3948/#review13201 -----------------------------------------------------------
I think it is kind of hard to review this patch knowing there are potential issues. I'd say that it'd be worthwhile to investigate Walter's potential memory leak before continuing the peer review of this patch. Beyond that, however, outboundproxy is a mine field. No one has really come to an agreement on what the defined behaviour should be when an outboundproxy is set on a peer - and while dealing with outboundproxy and OPTIONS requests/qualify is slightly less fraught than, say, registration or other scenarios, it bothers me that we're fixing bugs in something that doesn't have a definition of how it is supposed to work. For reference: Josh fixing outboundproxy (and breaking other things): https://reviewboard.asterisk.org/r/2851/ Josh asking for clarification on behaviour on the asterisk-dev list (and not getting much discussion): http://lists.digium.com/pipermail/asterisk-dev/2013-October/063312.html I'd really like to get some consensus on how users of outboundproxy expect it to behave in the scenarios uncovered by ttps://reviewboard.asterisk.org/r/2851/. Otherwise, it feels like we're chasing a moving target here, and that's not fun for anyone. - Matt Jordan On Aug. 25, 2014, 5:04 p.m., Damian Ivereigh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3948/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2014, 5:04 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24063 > https://issues.asterisk.org/jira/browse/ASTERISK-24063 > > > Repository: Asterisk > > > Description > ------- > > The outboundproxy setting is ignored when sending the qualify packets > (OPTIONS). This means that if an asterisk server is unable to send the packet > directly to a peer, it is unable to qualify any non inbound registered peer > (e.g. a peer SIP Trunk). This problem is found on asterisk-11.6-cert4 (and > many others) > > It has been pointed out (thanks Walter Doekes), that the p->outboundproxy may > not be freed at the end which would create a memory leak. > > > Diffs > ----- > > certified/tags/11.6-cert4/channels/chan_sip.c 422052 > > Diff: https://reviewboard.asterisk.org/r/3948/diff/ > > > Testing > ------- > > Have run this change in production for many months, however the possible > memory leak issue needs to be verified. > > > Thanks, > > Damian Ivereigh > >
-- _____________________________________________________________________ -- 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
