----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3587/#review12059 -----------------------------------------------------------
/branches/12/res/res_pjsip_notify.c <https://reviewboard.asterisk.org/r/3587/#comment22025> You've done a good job to plug some of the memory leaks that were previously present, but there are still some here. At a minimum, you'll need to call ast_variables_destroy(first_header) in the INVALID_ENDPOINT and ALLOC_ERROR cases in the switch statement. However, I think the addition of the first_header variable list is adding unnecessary allocations to the process. My suggestion is to do the following: 1) Get rid of the first_header and header variables. 2) Get rid of the for loop you added. If you want to warn about an unnecessary Content-Length header, you can add the check and warning message to the for loop in build_ami_notify(). IMO, the message shouldn't be a warning, but a notice. 3) Get rid of the ast_variables_destroy(vars) call you added above the switch statement, as well as the setting of vars to NULL afterwards. 4) Pass vars as the second argument to push_notify() 5) Call ast_variables_destroy(vars) in the INVALID_ENDPOINT and ALLOC_ERROR cases in the switch statement. - Mark Michelson On June 5, 2014, 6:19 p.m., Jonathan Rose wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3587/ > ----------------------------------------------------------- > > (Updated June 5, 2014, 6:19 p.m.) > > > Review request for Asterisk Developers and Mark Michelson. > > > Repository: Asterisk > > > Description > ------- > > There was some confusion on usage of the PJSIPNotify manager command... > notably in how headers are added to the notify message. This patch adds > documentation explaining how to add headers and also adds some of the > argument vetting from the chan_sip variant (SIPNotify). > > > Diffs > ----- > > /branches/12/res/res_pjsip_notify.c 415187 > > Diff: https://reviewboard.asterisk.org/r/3587/diff/ > > > Testing > ------- > > Send some notifies with in the documented manner. Added Content + > Content-type + Content-length variables to test how that worked. > > > 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
