Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.

2014-08-26 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3944/#review13186
---

Ship it!


Ship It!

- rmudgett


On Aug. 26, 2014, 5:43 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3944/
 ---
 
 (Updated Aug. 26, 2014, 5:43 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Although tab completion for confbridge mute and unmute show 'all' as a valid 
 channel target, it was never implemented.
 
 This patch updates mute and unmute (both CLI and AMI) to allow the 'all' 
 target.  They now work as kick does.  Since I was there, I made the channel 
 name case-insensitive since kick was already case-insensitive.
 
 
 Diffs
 -
 
   branches/12/apps/app_confbridge.c 422068 
 
 Diff: https://reviewboard.asterisk.org/r/3944/diff/
 
 
 Testing
 ---
 
 Tested that both CLI and AMI handle 'all' and 'participants' as a channel 
 target for mute, unmute and kick correctly.
 
 
 Thanks,
 
 George Joseph
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3928: res_musiconhold: Fix MOH restarting where it left off from the last hold.

2014-08-25 Thread rmudgett 


 On Aug. 24, 2014, 11:20 a.m., wdoekes wrote:
  /branches/1.8/res/res_musiconhold.c, line 421
  https://reviewboard.asterisk.org/r/3928/diff/1/?file=66739#file66739line421
 
  Something like this?
  
  /* If the class has the same name and amount of files, continue where 
  we left off. If not, reset state. */

That isn't a good comment as it just restates the code.  I'll add a comment 
giving the why.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3928/#review13159
---


On Aug. 22, 2014, 5:25 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3928/
 ---
 
 (Updated Aug. 22, 2014, 5:25 p.m.)
 
 
 Review request for Asterisk Developers and wdoekes.
 
 
 Bugs: ASTERISK-24019
 https://issues.asterisk.org/jira/browse/ASTERISK-24019
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Restore code removed by https://reviewboard.asterisk.org/r/3536/ that 
 introduced a regression that prevents MOH from restarting were it left off 
 the last time.
 
 I think the previous change removed the code because there was an incorrect 
 comment that indicated the dereferencing of a stale MOH class pointer.
 
 
 Diffs
 -
 
   /branches/1.8/res/res_musiconhold.c 421902 
 
 Diff: https://reviewboard.asterisk.org/r/3928/diff/
 
 
 Testing
 ---
 
 Establish a call between two parties and place one party on and off hold 
 several times.
 MOH resumes where it left off from the last hold.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3928: res_musiconhold: Fix MOH restarting where it left off from the last hold.

2014-08-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3928/
---

(Updated Aug. 25, 2014, 4 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and wdoekes.


Changes
---

Committed in revision 421976


Bugs: ASTERISK-24019
https://issues.asterisk.org/jira/browse/ASTERISK-24019


Repository: Asterisk


Description
---

Restore code removed by https://reviewboard.asterisk.org/r/3536/ that 
introduced a regression that prevents MOH from restarting were it left off the 
last time.

I think the previous change removed the code because there was an incorrect 
comment that indicated the dereferencing of a stale MOH class pointer.


Diffs
-

  /branches/1.8/res/res_musiconhold.c 421902 

Diff: https://reviewboard.asterisk.org/r/3928/diff/


Testing
---

Establish a call between two parties and place one party on and off hold 
several times.
MOH resumes where it left off from the last hold.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3929: ARI: Holding bridge issues with bridge Music on Hold, playback operations, and default channel roles

2014-08-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3929/#review13171
---



/branches/12/res/ari/resource_bridges.c
https://reviewboard.asterisk.org/r/3929/#comment23629

Remove.  This is redundant and too late anyway.  The announcer role is 
already added to the ;2 channel as part of the setup by the Announcer channel 
technology in chan_bridge_media.c.



/branches/12/res/ari/resource_bridges.c
https://reviewboard.asterisk.org/r/3929/#comment23628

Remove.  This is redundant and too late anyway.  The announcer role is 
already added to the ;2 channel as part of the setup by the Announcer channel 
technology in chan_bridge_media.c.



/branches/12/res/stasis/stasis_bridge.c
https://reviewboard.asterisk.org/r/3929/#comment23631

/*
 * Block
 * comments
 */



/branches/12/res/stasis/stasis_bridge.c
https://reviewboard.asterisk.org/r/3929/#comment23632

Use ast_channel_has_role() instead of ast_bridge_channel_has_role().  You 
cannot use ast_bridge_channel_has_role() because the bridge_channel has not had 
the roles established from the channel yet.



/branches/12/res/stasis/stasis_bridge.c
https://reviewboard.asterisk.org/r/3929/#comment23630

Why is this being done?  For MOH or playback channels they are immovable 
and can only be hungup when they leave the bridge.

If anything, this should be done when the channel is moved out of stasis to 
a non-stasis controlled bridge as part of the move hook.


- rmudgett


On Aug. 22, 2014, 11:13 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3929/
 ---
 
 (Updated Aug. 22, 2014, 11:13 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24264
 https://issues.asterisk.org/jira/browse/ASTERISK-24264
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When ARI manipulates a bridge, it generally doesn't care what the mixing 
 technology is. Operations should initiate on the bridge regardless of its 
 mixing technology - and while that mixing technology may determine the 
 experience channels within the bridge have, the actual operations themselves 
 should be the same.
 
 This isn't the case with holding bridges. Currently, the following issues 
 exist:
  * Music on Hold is played into a bridge using an Announcer channel. There 
 are two issues with it currently:
(1) The Music on Hold channel is also not marked as being allowed within a 
 Stasis bridge, so it generally never makes it into the bridge (of any type).
(2) Even if it did, because it does not have a bridge role of announcer, 
 it joins the holding bridge as a participant. Additionally, the holding 
 bridge starts MoH on the Announcer channel (which is ironic, but not super 
 useful).
  * Playback channels do not join as an announcer. Playing back announcements 
 using the /play operation would not be heard by participants.
  * Participants join without a role. As such, MoH is started for the channels 
 automatically; however, subsequent bridge operations that would stop MoH 
 would fail (as there is no Announcer channel playing MoH to the bridge). From 
 the perspective of ARI users, this is counter-intuitive - I would not expect 
 MoH to be started for me. The mixing technology determines how media is 
 shared between participants, not the application experience.
 
 This patch does the following:
  * The Stasis bridge class now inspects channels as they are going into a 
 bridge. If the bridge has a holding capability, and the channel has no roles, 
 we give it a participant role and mark the default behaviour to have no 
 entertainment. This allows addChannel operations to continue to set a 
 participant role with an entertainment option if it felt like it (or could do 
 it).
  * The playback channel now gets a role of announcer, as does the music on 
 hold channel. For mixing technologies this is a NoOp; for holding bridges it 
 means the channels should have the expected behaviour.
  * The music on hold channel is now Stasis approved (tm)
  * Finally, a small bug in 'core show channel' was observed, in that we 
 attempted to calculate CDR variables for internal channels. That generates an 
 annoying warning; internal channels now no longer attempt to access CDR data.
 
 
 Diffs
 -
 
   /branches/12/res/stasis/stasis_bridge.c 421906 
   /branches/12/res/res_stasis.c 421906 
   /branches/12/res/ari/resource_bridges.c 421906 
   /branches/12/main/cli.c 421906 
 
 Diff: https://reviewboard.asterisk.org/r/3929/diff/
 
 
 Testing
 ---
 
 The bridge-infinite-wait examples on the wiki (that Sam and I are working on) 
 now ... work.
 
 
 Thanks,
 
 Matt Jordan

Re: [asterisk-dev] [Code Review] 3923: CallerID: Fix parsing of malformed callerid strings

2014-08-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3923/#review13173
---



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23636

static const struct cid_set cid_sets[]



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23635

static const struct cid_set cid_sets[]



branches/1.8/tests/test_utils.c
https://reviewboard.asterisk.org/r/3923/#comment23634

static const struct quote_set escape_sets[]



branches/1.8/tests/test_utils.c
https://reviewboard.asterisk.org/r/3923/#comment23633

static const struct quote_set escape_sets[]


- rmudgett


On Aug. 25, 2014, 8:52 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3923/
 ---
 
 (Updated Aug. 25, 2014, 8:52 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This allows the callerid parsing function to handle malformed input strings 
 and strings containing escaped and unescaped double quotes. This also adds a 
 unittest to cover many of the cases where the parsing algorithm previously 
 failed.
 
 
 Diffs
 -
 
   branches/1.8/tests/test_utils.c 421534 
   branches/1.8/tests/test_callerid.c PRE-CREATION 
   branches/1.8/main/utils.c 421534 
   branches/1.8/main/callerid.c 421534 
   branches/1.8/include/asterisk/utils.h 421534 
   branches/1.8/channels/chan_sip.c 421534 
 
 Diff: https://reviewboard.asterisk.org/r/3923/diff/
 
 
 Testing
 ---
 
 Ran the unittest.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3923: CallerID: Fix parsing of malformed callerid strings

2014-08-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3923/#review13174
---



branches/1.8/main/callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23637

Where did the requirement to allow a missing  come from?  The previous 
code required the  to match.  The prevous code was more lenient with the 
quotes.



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23639

I'd prefer that cid_set[] did not have the same name as the struct.



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23638

AST-158 explicitly needs to be tested for:
\name number\, name, number

I'd prefer that cid_set[] did not have the same name as the struct.


- rmudgett


On Aug. 25, 2014, 3:50 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3923/
 ---
 
 (Updated Aug. 25, 2014, 3:50 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This allows the callerid parsing function to handle malformed input strings 
 and strings containing escaped and unescaped double quotes. This also adds a 
 unittest to cover many of the cases where the parsing algorithm previously 
 failed.
 
 
 Diffs
 -
 
   branches/1.8/tests/test_utils.c 421975 
   branches/1.8/tests/test_callerid.c PRE-CREATION 
   branches/1.8/main/utils.c 421975 
   branches/1.8/main/callerid.c 421975 
   branches/1.8/include/asterisk/utils.h 421975 
   branches/1.8/channels/chan_sip.c 421975 
 
 Diff: https://reviewboard.asterisk.org/r/3923/diff/
 
 
 Testing
 ---
 
 Ran the unittest.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.

2014-08-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3944/#review13176
---


I don't think a mute/unmute all feature was intended when the kick all feature 
was added.  The mute/unmute commands call the same completion routine and thus 
unintentionally got the all added to the list.

However, you might want to make the mute/unmute all set the mute only on 
non-admin users.  Otherwise the conference would be rather dull with everyone 
muted.


branches/12/apps/app_confbridge.c
https://reviewboard.asterisk.org/r/3944/#comment23640

This is set but not used since it is set to -2 by the next use.


- rmudgett


On Aug. 25, 2014, 4:43 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3944/
 ---
 
 (Updated Aug. 25, 2014, 4:43 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Although tab completion for confbridge mute and unmute show 'all' as a valid 
 channel target, it was never implemented.
 
 This patch updates mute and unmute (both CLI and AMI) to allow the 'all' 
 target.  They now work as kick does.  Since I was there, I made the channel 
 name case-insensitive since kick was already case-insensitive.
 
 
 Diffs
 -
 
   branches/12/apps/app_confbridge.c 422052 
 
 Diff: https://reviewboard.asterisk.org/r/3944/diff/
 
 
 Testing
 ---
 
 Tested that both CLI and AMI handle 'all' as a channel target for mute and 
 unmute correctly.
 
 
 Thanks,
 
 George Joseph
 


-- 
_
-- 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

[asterisk-dev] [Code Review] 3928: res_musiconhold: Fix MOH restarting where it left off from the last hold.

2014-08-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3928/
---

Review request for Asterisk Developers and wdoekes.


Bugs: ASTERISK-24019
https://issues.asterisk.org/jira/browse/ASTERISK-24019


Repository: Asterisk


Description
---

Restore code removed by https://reviewboard.asterisk.org/r/3536/ that 
introduced a regression that prevents MOH from restarting were it left off the 
last time.

I think the previous change removed the code because there was an incorrect 
comment that indicated the dereferencing of a stale MOH class pointer.


Diffs
-

  /branches/1.8/res/res_musiconhold.c 421902 

Diff: https://reviewboard.asterisk.org/r/3928/diff/


Testing
---

Establish a call between two parties and place one party on and off hold 
several times.
MOH resumes where it left off from the last hold.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3927: Resolve race condition in scheduler when attempting to delete a running task

2014-08-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3927/#review13156
---



/branches/12/main/sched.c
https://reviewboard.asterisk.org/r/3927/#comment23619

Initing cond must be done only once so it should be done in sched_alloc().  
sched_alloc() either creates a new sched object or gets it from a cache.


- rmudgett


On Aug. 22, 2014, 2:39 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3927/
 ---
 
 (Updated Aug. 22, 2014, 2:39 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24212
 https://issues.asterisk.org/jira/browse/ASTERISK-24212
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Several tests in the testsuite had sporadic failures due to crashes that were 
 occurring due to the scheduler. The crash goes something like this:
 
 1) Scheduler thread realizes it's time to send an RTCP packet.
 2) Scheduler thread removes RTCP task from the heap so that it can be run.
 3) A separate thread ends a call in progress, and attempts to delete the RTCP 
 scheduler task using ast_sched_del().
 4) ast_sched_del() cannot find the scheduled task since it is not in the heap 
 (or hashtab in Asterisk 12). This results in a failed assertion.
 5) Since the test agents are compiled with DO_CRASH, failing an assertion 
 results in a crash.
 6) A crash results in a failed test.
 
 The solution I have crafted here is to maintain a pointer in the scheduler 
 context to which task is currently executing. If we attempt to delete the 
 running task, we wait for it to complete before continuing and return that we 
 successfully deleted the scheduled task.
 
 
 Diffs
 -
 
   /branches/12/main/sched.c 421883 
 
 Diff: https://reviewboard.asterisk.org/r/3927/diff/
 
 
 Testing
 ---
 
 The test 
 channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hangs_up 
 was a test that, when I ran it in a loop, would have a test failure typically 
 within about a half hour of starting the test loop. With this patch applied, 
 I no longer see the crash described in the description.
 
 HOWEVER, the test still does occasionally fail, but that's due to a separate 
 race condition involving translation paths not being set up when attempting 
 to perform talk detection. So while the patch attached here may not 
 necessarily be enough to close the referenced issue, it is fixing one of the 
 reasons for test failure.
 
 
 Thanks,
 
 Mark Michelson
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3908: ARI: Fix bug where hanging up while in a bridge during playback causes a crash

2014-08-21 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3908/#review13141
---


mostly minor nits


/branches/12/include/asterisk/stasis_app_impl.h
https://reviewboard.asterisk.org/r/3908/#comment23596

stasis_app_send_command



/branches/12/res/res_stasis.c
https://reviewboard.asterisk.org/r/3908/#comment23597

blank line not needed since the needs_depart is set to test it here.



/branches/12/res/res_stasis_playback.c
https://reviewboard.asterisk.org/r/3908/#comment23599

playback is not checked for NULL from create failure.



/branches/12/res/res_stasis_playback.c
https://reviewboard.asterisk.org/r/3908/#comment23598

To be safe, swap the ao2_ref and the send_comand statements.  If the 
send_command fails to send, playback is unreffed thus this function may no 
longer have a valid ref to playback.

The playback RAII_VAR here looks to be another silly useage.

The game played with the playback ref here is rather fragile.  A playback 
ref should be passed to the play_uri command and cleaned up by the 
remove_from_playback destructor as well as unlinking playback from the 
playbacks container.  What happens if playback fails to get linked into the 
playbacks container a few lines earlier?



/branches/12/res/res_stasis_recording.c
https://reviewboard.asterisk.org/r/3908/#comment23600

The same fragile game with recording is being played here as with playback 
pointed out elsewhere though the recording RAII_VAR is more useful here.  What 
happens if recording fails to get linked into the recordings container a few 
lines above?



/branches/12/res/stasis/command.h
https://reviewboard.asterisk.org/r/3908/#comment23602

stray blank line change from removing the wrapper functions.



/branches/12/res/stasis/command.c
https://reviewboard.asterisk.org/r/3908/#comment23603

Silly RAII_VAR usage.



/branches/12/res/stasis/control.c
https://reviewboard.asterisk.org/r/3908/#comment23606

The line wasn't long enough to require a wrap.



/branches/12/res/stasis/control.c
https://reviewboard.asterisk.org/r/3908/#comment23607

The line wasn't long enough to require a wrap.



/branches/12/res/stasis/stasis_bridge.c
https://reviewboard.asterisk.org/r/3908/#comment23608

Line should be wrapped before ao2_bump.


- rmudgett


On Aug. 21, 2014, 11:30 a.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3908/
 ---
 
 (Updated Aug. 21, 2014, 11:30 a.m.)
 
 
 Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
 
 
 Bugs: ASTERISK-24147
 https://issues.asterisk.org/jira/browse/ASTERISK-24147
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Here's a basic rundown of what was happening:
 
 1. Stasis application with a channel in it, channel is in a bridge.  Playback 
 actions for the channel are queued, the stasis control is running them.
 
 2. The channel hangs up. Because the stasis control detects a hangup and the 
 depart is expected to be handled when the channel leaves the bridge, the 
 stasis application execution function exits taking the control along with it. 
 When the PBX thread exits, the channel is considered fully hung up and leaves 
 the bridging core. At this point we are running after bridge callbacks with, 
 one of which holds a pointer to a dead control object that we are trying to 
 do all queue actions to along with other things.  This causes the crash.
 
 In order to fix this, I made sure that if the control execution loop is 
 exited without pulling the channel out of the bridge that the channel depart 
 would occur manually and the control would be marked so that the after bridge 
 cb wouldn't attempt to exit the bridge in this case. This wasn't actually 
 causing problems, but Richard and I both have concerns about an after bridge 
 callback attempting to depart a channel from a bridge... it seems a little 
 out there.
 
 
 Diffs
 -
 
   /branches/12/res/stasis/stasis_bridge.c 421424 
   /branches/12/res/stasis/control.c 421424 
   /branches/12/res/stasis/command.c 421424 
   /branches/12/res/stasis/command.h 421424 
   /branches/12/res/res_stasis_recording.c 421424 
   /branches/12/res/res_stasis_playback.c 421424 
   /branches/12/res/res_stasis_answer.c 421424 
   /branches/12/res/res_stasis.c 421424 
   /branches/12/include/asterisk/stasis_app_impl.h 421424 
 
 Diff: https://reviewboard.asterisk.org/r/3908/diff/
 
 
 Testing
 ---
 
 Made sure the crash that was happening no longer occurs
 
 Tested long queues of playback to check what would happen with the additional 
 playbacks.  Each subsequent playback from the one currently running fails (in 
 an expected

Re: [asterisk-dev] [Code Review] 3908: ARI: Fix bug where hanging up while in a bridge during playback causes a crash

2014-08-21 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3908/#review13152
---



/branches/12/res/res_stasis_recording.c
https://reviewboard.asterisk.org/r/3908/#comment23616

Missed an unref recoring here.

You really didn't need to eliminate the RAII_VAR here because there were 
multiple return points that would have a recording to cleanup.  but oh well.


- rmudgett


On Aug. 21, 2014, 2:44 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3908/
 ---
 
 (Updated Aug. 21, 2014, 2:44 p.m.)
 
 
 Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
 
 
 Bugs: ASTERISK-24147
 https://issues.asterisk.org/jira/browse/ASTERISK-24147
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Here's a basic rundown of what was happening:
 
 1. Stasis application with a channel in it, channel is in a bridge.  Playback 
 actions for the channel are queued, the stasis control is running them.
 
 2. The channel hangs up. Because the stasis control detects a hangup and the 
 depart is expected to be handled when the channel leaves the bridge, the 
 stasis application execution function exits taking the control along with it. 
 When the PBX thread exits, the channel is considered fully hung up and leaves 
 the bridging core. At this point we are running after bridge callbacks with, 
 one of which holds a pointer to a dead control object that we are trying to 
 do all queue actions to along with other things.  This causes the crash.
 
 In order to fix this, I made sure that if the control execution loop is 
 exited without pulling the channel out of the bridge that the channel depart 
 would occur manually and the control would be marked so that the after bridge 
 cb wouldn't attempt to exit the bridge in this case. This wasn't actually 
 causing problems, but Richard and I both have concerns about an after bridge 
 callback attempting to depart a channel from a bridge... it seems a little 
 out there.
 
 
 Diffs
 -
 
   /branches/12/res/stasis/stasis_bridge.c 421424 
   /branches/12/res/stasis/control.c 421424 
   /branches/12/res/stasis/command.c 421424 
   /branches/12/res/stasis/command.h 421424 
   /branches/12/res/res_stasis_recording.c 421424 
   /branches/12/res/res_stasis_playback.c 421424 
   /branches/12/res/res_stasis_answer.c 421424 
   /branches/12/res/res_stasis.c 421424 
   /branches/12/include/asterisk/stasis_app_impl.h 421424 
 
 Diff: https://reviewboard.asterisk.org/r/3908/diff/
 
 
 Testing
 ---
 
 Made sure the crash that was happening no longer occurs
 
 Tested long queues of playback to check what would happen with the additional 
 playbacks.  Each subsequent playback from the one currently running fails (in 
 an expected manner) and reports its failure over stasis.  Nothing too odd 
 there.
 
 
 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

Re: [asterisk-dev] [Code Review] 3908: ARI: Fix bug where hanging up while in a bridge during playback causes a crash

2014-08-21 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3908/#review13154
---

Ship it!



/branches/12/res/res_stasis_recording.c
https://reviewboard.asterisk.org/r/3908/#comment23618

Remove now inaccurate comment.


- rmudgett


On Aug. 21, 2014, 3:11 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3908/
 ---
 
 (Updated Aug. 21, 2014, 3:11 p.m.)
 
 
 Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
 
 
 Bugs: ASTERISK-24147
 https://issues.asterisk.org/jira/browse/ASTERISK-24147
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Here's a basic rundown of what was happening:
 
 1. Stasis application with a channel in it, channel is in a bridge.  Playback 
 actions for the channel are queued, the stasis control is running them.
 
 2. The channel hangs up. Because the stasis control detects a hangup and the 
 depart is expected to be handled when the channel leaves the bridge, the 
 stasis application execution function exits taking the control along with it. 
 When the PBX thread exits, the channel is considered fully hung up and leaves 
 the bridging core. At this point we are running after bridge callbacks with, 
 one of which holds a pointer to a dead control object that we are trying to 
 do all queue actions to along with other things.  This causes the crash.
 
 In order to fix this, I made sure that if the control execution loop is 
 exited without pulling the channel out of the bridge that the channel depart 
 would occur manually and the control would be marked so that the after bridge 
 cb wouldn't attempt to exit the bridge in this case. This wasn't actually 
 causing problems, but Richard and I both have concerns about an after bridge 
 callback attempting to depart a channel from a bridge... it seems a little 
 out there.
 
 
 Diffs
 -
 
   /branches/12/res/stasis/stasis_bridge.c 421424 
   /branches/12/res/stasis/control.c 421424 
   /branches/12/res/stasis/command.c 421424 
   /branches/12/res/stasis/command.h 421424 
   /branches/12/res/res_stasis_recording.c 421424 
   /branches/12/res/res_stasis_playback.c 421424 
   /branches/12/res/res_stasis_answer.c 421424 
   /branches/12/res/res_stasis.c 421424 
   /branches/12/include/asterisk/stasis_app_impl.h 421424 
 
 Diff: https://reviewboard.asterisk.org/r/3908/diff/
 
 
 Testing
 ---
 
 Made sure the crash that was happening no longer occurs
 
 Tested long queues of playback to check what would happen with the additional 
 playbacks.  Each subsequent playback from the one currently running fails (in 
 an expected manner) and reports its failure over stasis.  Nothing too odd 
 there.
 
 
 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

Re: [asterisk-dev] [Code Review] 3926: sip.conf: Clarify that sipdebug=yes cannot be undone by the CLI

2014-08-21 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3926/#review13155
---

Ship it!


Ship It!

- rmudgett


On Aug. 21, 2014, 5:45 p.m., wdoekes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3926/
 ---
 
 (Updated Aug. 21, 2014, 5:45 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24249
 https://issues.asterisk.org/jira/browse/ASTERISK-24249
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 It's documented in the source:
 
 /*! \brief debugging state
  * We store separately the debugging requests from the config file
  * and requests from the CLI. Debugging is enabled if either is set
  * (which means that if sipdebug is set in the config file, we can
  * only turn it off by reloading the config).
  */
 
 but it wasn't in sip.conf.
 
 
 Diffs
 -
 
   /branches/1.8/configs/sip.conf.sample 420566 
 
 Diff: https://reviewboard.asterisk.org/r/3926/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 wdoekes
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3906: chan_pjsip: Update media translation paths when new SDP negotiated.

2014-08-20 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3906/
---

(Updated Aug. 20, 2014, 5:49 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 421645


Bugs: AFS-137
https://issues.asterisk.org/jira/browse/AFS-137


Repository: Asterisk


Description
---

On a SIP reinvite that changes media strams, the PJSIP channel driver was 
flooding the log with Asked to transmit frame type %s, while native formats is 
%s warnings.

* Fixes PJSIP not setting up translation paths when the formats change on a 
reinvite.

* Improved the unexpected frame format WARNING message to include more 
information.

* Added protective locking while altering formats on a channel.  Reworked 
set_format() to simplify and protect the formats under manipulation.

* Restored some code that got lost in the media_formats work.


Diffs
-

  /branches/13/res/res_pjsip_sdp_rtp.c 420978 
  /branches/13/main/file.c 420978 
  /branches/13/main/channel.c 420978 
  /branches/13/main/bridge_channel.c 420978 
  /branches/13/main/bridge.c 420978 
  /branches/13/channels/chan_pjsip.c 420978 

Diff: https://reviewboard.asterisk.org/r/3906/diff/


Testing
---

Performed blind SIP transfers (REFER) and the call still passes media.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3908: ARI: Fix bug where hanging up while in a bridge during playback causes a crash

2014-08-20 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3908/#review13128
---



/branches/12/res/stasis/command.h
https://reviewboard.asterisk.org/r/3908/#comment23589

Remove this special function and use ast_free_ptr instead.  This is what 
ast_free_ptr is for.



/branches/12/res/stasis/command.h
https://reviewboard.asterisk.org/r/3908/#comment23588

Remove this special function and use __ao2_cleanup instead.  This is what 
__ao2_cleanup is for.



/branches/12/res/stasis/command.c
https://reviewboard.asterisk.org/r/3908/#comment23592

You should call the data_destructor here if it is present and then clear 
the data_destructor pointer.  Otherwise, you are extending the life of the data 
object possibly too long from what it was before.



/branches/12/res/stasis/command.c
https://reviewboard.asterisk.org/r/3908/#comment23591

Call the data_destructor here if there is one to guarantee that the data 
will be destroyed regardless of successfully queueing the command.  Otherwise, 
you could return -1 and the caller would not know if the data was destroyed.

It might be better to have command_create() do this instead if the command 
object could not be created.  This way the other places that create commands 
would be covered as well.


- rmudgett


On Aug. 20, 2014, 4:21 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3908/
 ---
 
 (Updated Aug. 20, 2014, 4:21 p.m.)
 
 
 Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
 
 
 Bugs: ASTERISK-24147
 https://issues.asterisk.org/jira/browse/ASTERISK-24147
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Here's a basic rundown of what was happening:
 
 1. Stasis application with a channel in it, channel is in a bridge.  Playback 
 actions for the channel are queued, the stasis control is running them.
 
 2. The channel hangs up. Because the stasis control detects a hangup and the 
 depart is expected to be handled when the channel leaves the bridge, the 
 stasis application execution function exits taking the control along with it. 
 When the PBX thread exits, the channel is considered fully hung up and leaves 
 the bridging core. At this point we are running after bridge callbacks with, 
 one of which holds a pointer to a dead control object that we are trying to 
 do all queue actions to along with other things.  This causes the crash.
 
 In order to fix this, I made sure that if the control execution loop is 
 exited without pulling the channel out of the bridge that the channel depart 
 would occur manually and the control would be marked so that the after bridge 
 cb wouldn't attempt to exit the bridge in this case. This wasn't actually 
 causing problems, but Richard and I both have concerns about an after bridge 
 callback attempting to depart a channel from a bridge... it seems a little 
 out there.
 
 
 Diffs
 -
 
   /branches/12/res/stasis/stasis_bridge.c 421424 
   /branches/12/res/stasis/control.c 421424 
   /branches/12/res/stasis/command.c 421424 
   /branches/12/res/stasis/command.h 421424 
   /branches/12/res/res_stasis_recording.c 421424 
   /branches/12/res/res_stasis_playback.c 421424 
   /branches/12/res/res_stasis_answer.c 421424 
   /branches/12/res/res_stasis.c 421424 
   /branches/12/include/asterisk/stasis_app_impl.h 421424 
 
 Diff: https://reviewboard.asterisk.org/r/3908/diff/
 
 
 Testing
 ---
 
 Made sure the crash that was happening no longer occurs
 
 Tested long queues of playback to check what would happen with the additional 
 playbacks.  Each subsequent playback from the one currently running fails (in 
 an expected manner) and reports its failure over stasis.  Nothing too odd 
 there.
 
 
 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

Re: [asterisk-dev] [Code Review] 3913: chan_pjsip: Attended transfer does not update connected line name.

2014-08-19 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3913/
---

(Updated Aug. 19, 2014, 11:07 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 421400


Bugs: AFS-98
https://issues.asterisk.org/jira/browse/AFS-98


Repository: Asterisk


Description
---

A calls B
B SIP attended transfers to C
C answers, B and C can see each other's connected line information
B completes the transfer
A has number but no name connected line information about C while C has the 
full information about A

I examined the incoming and outgoing party id information handling of 
chan_pjsip and found several issues:

* Fixed ast_sip_session_create_outgoing() not setting up the configured 
endpoint id as the new channel's caller id.  This is why party A got default 
connected line information.

* Made update_initial_connected_line() use the channel's CALLERID(id) 
information.  The core, app_dial, or predial routine may have filled in or 
changed the endpoint caller id information.

* Fixed chan_pjsip_new() not setting the full party id information available on 
the caller id and ANI party id.  This includes the configured callerid_tag 
string and other party id fields.

* Fixed accessing channel party id information without the channel lock held.

* Fixed using the effective connected line id without doing a deep copy outside 
of holding the channel lock.  Shallow copy string pointers can become stale if 
the channel lock is not held.

* Made queue_connected_line_update() also update the channel's CALLERID(id) 
information.  Moving the channel to another bridge would need the information 
there for the new bridge peer.

* Fixed off nominal memory leak in update_incoming_connected_line().

* Added callerid_tag string to party id information from enabled trust_inbound 
endpoint in caller_id_incoming_request().


Diffs
-

  /branches/13/res/res_pjsip_session.c 421122 
  /branches/13/res/res_pjsip_caller_id.c 421122 
  /branches/13/channels/chan_pjsip.c 421122 

Diff: https://reviewboard.asterisk.org/r/3913/diff/


Testing
---

Attended transfer gives correct party id information to all parties involved.
Blind transfer gives correct party id information to all parties involved.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3920: Fix after bridge behavior when channel moves from a Stasis bridge to a non-Stasis bridge

2014-08-19 Thread rmudgett 


 On Aug. 19, 2014, 12:29 p.m., opticron wrote:
  /branches/13/res/stasis/control.c, lines 824-826
  https://reviewboard.asterisk.org/r/3920/diff/2/?file=66624#file66624line824
 
  Drop the explicit locks and use ast_softhangup() instead since it locks 
  chan for you.

Actually, those two functions differ more than just locking the channel.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3920/#review13118
---


On Aug. 19, 2014, 11:10 a.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3920/
 ---
 
 (Updated Aug. 19, 2014, 11:10 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a channel is moved from a Stasis bridge to a non-Stasis bridge, the 
 behavior after the non-Stasis bridge dissolves is incorrect. The issue is 
 that since channels are imparted into Stasis bridges as departable rather 
 than independent, control of the channel returns to the main Stasis 
 application loop after the non-Stasis bridge dissolves. From the end-user 
 perspective, this is most odd.
 
 As an example, say that Alice calls into Stasis and is placed into a Stasis 
 bridge. Bob places a call into the dialplan and calls Bridge(Alice,x), 
 requesting to be bridged with Alice and requesting that Alice be hung up if 
 Bob hangs up first. Alice is pulled from the Stasis, is sent a StasisEnd 
 event, and is pushed into the non-Stasis bridge created by the Bridge 
 application. If Bob were to hang up, the expectation would be that Alice's 
 channel would be hung up as well. However, in actuality, Alice remains in the 
 Stasis application and must be hung up manually. Expecations of Alice's 
 post-bridge destination are also not met when passing the 'F' option or no 
 options at all to the Bridge application.
 
 This patch aims to correct the unexpected behavior by detecting the 
 circumstances when Alice's channel leaves the bridging system. If Alice has 
 already had a StasisEnd published when leaving the bridging system, then 
 Stasis's after bridge callback will attempt to direct Alice to where she is 
 intended to go.
 
 To be honest, I'm not a huge fan of this patch, but it gets the job done. The 
 proper way to fix the issue is to devise a method such that channels that 
 enter Stasis bridges are not departable. However, such a change is of larger 
 scope and risk than is acceptable for Asterisk 12 or 13 (in my judgment 
 anyway), so I have gone with the patch seen here. For Asterisk 14, I would 
 recommend a mini-project to make channels in Stasis bridges independent so 
 that the correct behavior is taken care of innately by the bridging system 
 instead.
 
 
 Diffs
 -
 
   /branches/13/res/stasis/control.c 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3920/diff/
 
 
 Testing
 ---
 
 I created a small ARI application that places any channel that enters the app 
 into a Stasis bridge. I then had a second channel call an extension in the 
 dialplan that called the Bridge application to move the original channel into 
 a non-Stasis bridge. I tested with several options passed to the Bridge 
 application. With the patch, the following occurred:
 
 Bridge(Alice): Channel re-entered Stasis when the non-Stasis bridge dissolved.
 Bridge(Alice,F): Channel moved to the priority after the Bridge() application 
 when the non-Stasis bridge dissolved.
 Bridge(Alice,x): Channel was hung up after the non-Stasis bridge dissolved.
 
 
 Thanks,
 
 Mark Michelson
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3923: CallerID: Fix parsing of malformed callerid strings

2014-08-19 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3923/#review13120
---



branches/1.8/main/callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23574

This feature of quote_count is effectively dead code as you never pass -1.



branches/1.8/main/callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23575

I am wondering if ast_callerid_parse() should be escaping the quotes at 
all.  The job of ast_callerid_parse() is to take the string and separate it 
into a name and number string.  Escaping characters seems wrong.  If anything, 
the routine should be unescaping character sequences.  The consumers likely 
don't know how to handle escaped quotes or may need to escape characters 
differently.



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23576

red



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23577

red



branches/1.8/tests/test_callerid.c
https://reviewboard.asterisk.org/r/3923/#comment23573

Rather than a bunch of redundant tests checking minor variations in the 
string.  How about one test that works through an array of input string 
variatons and the expected broken out strings:
struct expect {
  char *orig;
  char *name;
  char *number;
};
struct expect array[] = {
  { name number, name, number },
  { \name\ number, name, number },
  { name, name, NULL },
  { 1234, NULL, 1234 }
}

The last two entries above are also valid possibilities for the input 
string to ast_callerid_parse().


- rmudgett


On Aug. 18, 2014, 8:55 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3923/
 ---
 
 (Updated Aug. 18, 2014, 8:55 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This allows the callerid parsing function to handle malformed input strings 
 and strings containing escaped and unescaped double quotes. This also adds a 
 unittest to cover many of the cases where the parsing algorithm previously 
 failed.
 
 
 Diffs
 -
 
   branches/1.8/tests/test_callerid.c PRE-CREATION 
   branches/1.8/res/res_agi.c 421326 
   branches/1.8/main/privacy.c 421326 
   branches/1.8/main/manager.c 421326 
   branches/1.8/main/callerid.c 421326 
   branches/1.8/include/asterisk/callerid.h 421326 
   branches/1.8/channels/chan_unistim.c 421326 
   branches/1.8/channels/chan_misdn.c 421326 
   branches/1.8/apps/app_voicemail.c 421326 
   branches/1.8/apps/app_dial.c 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3923/diff/
 
 
 Testing
 ---
 
 Ran the unittest.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3915: AMI: Add AllVariables parameter to Status

2014-08-19 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3915/#review13121
---

Ship it!


Minor existing documentation problem.


trunk/main/manager.c
https://reviewboard.asterisk.org/r/3915/#comment23578

Channel is not required.  If not supplied then all channels are to be 
processed.


- rmudgett


On Aug. 19, 2014, 7:16 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3915/
 ---
 
 (Updated Aug. 19, 2014, 7:16 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds the AllVariables parameter to the Status AMI action such that if 
 defined and set to true, all channel variables will be reported in the 
 subsequent Status event(s). This parameter does not negate the functionality 
 of the Variables parameter so that global variables and dialplan functions 
 can be requested.
 
 
 Diffs
 -
 
   trunk/main/manager.c 421392 
 
 Diff: https://reviewboard.asterisk.org/r/3915/diff/
 
 
 Testing
 ---
 
 Manual testing via netcat and an automated test here: 
 https://reviewboard.asterisk.org/r/3916/
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3921: Stasis: Add missing information to blind transfer events

2014-08-19 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3921/#review13122
---

Ship it!


Looks ok to me.

- rmudgett


On Aug. 19, 2014, 7:29 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3921/
 ---
 
 (Updated Aug. 19, 2014, 7:29 a.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a blind transfer occurs that is forced to create a local channel pair to 
 satisfy the transfer request, information about the local channel pair is not 
 published. This adds a field to describe that channel to the blind transfer 
 message struct so that this information is conveyed properly to consumers of 
 the blind transfer message.
 
 This also fixes a bug in which Stasis() was unable to properly identify the 
 channel that was replacing an existing Stasis-controlled channel due to a 
 blind transfer.
 
 
 Diffs
 -
 
   branches/12/rest-api/api-docs/events.json 421326 
   branches/12/res/stasis/app.c 421326 
   branches/12/res/ari/ari_model_validators.c 421326 
   branches/12/res/ari/ari_model_validators.h 421326 
   branches/12/main/stasis_bridges.c 421326 
   branches/12/main/bridge.c 421326 
   branches/12/include/asterisk/stasis_bridges.h 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3921/diff/
 
 
 Testing
 ---
 
 Verified functionality manually and updated the ARI blind transfer test to 
 check for these conditions.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3908: ARI: Fix bug where hanging up while in a bridge during playback causes a crash

2014-08-18 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3908/#review13103
---



/branches/12/res/res_stasis.c
https://reviewboard.asterisk.org/r/3908/#comment23553

This will not tell you if the channel is actually still in a bridge.  The 
/continue request will have bridge non-NULL.

Also the command_queue has no way of discarding any pending commands 
without leaking memory or ao2 objects since the command data could be either 
malloced memory or an ao2 object.


- rmudgett


On Aug. 14, 2014, 1:46 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3908/
 ---
 
 (Updated Aug. 14, 2014, 1:46 p.m.)
 
 
 Review request for Asterisk Developers, kmoore, Matt Jordan, and rmudgett.
 
 
 Bugs: ASTERISK-24147
 https://issues.asterisk.org/jira/browse/ASTERISK-24147
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Here's a basic rundown of what was happening:
 
 1. Stasis application with a channel in it, channel is in a bridge.  Playback 
 actions for the channel are queued, the stasis control is running them.
 
 2. The channel hangs up. Because the stasis control detects a hangup and the 
 depart is expected to be handled when the channel leaves the bridge, the 
 stasis application execution function exits taking the control along with it. 
 When the PBX thread exits, the channel is considered fully hung up and leaves 
 the bridging core. At this point we are running after bridge callbacks with, 
 one of which holds a pointer to a dead control object that we are trying to 
 do all queue actions to along with other things.  This causes the crash.
 
 In order to fix this, I made sure that if the control execution loop is 
 exited without pulling the channel out of the bridge that the channel depart 
 would occur manually and the control would be marked so that the after bridge 
 cb wouldn't attempt to exit the bridge in this case. This wasn't actually 
 causing problems, but Richard and I both have concerns about an after bridge 
 callback attempting to depart a channel from a bridge... it seems a little 
 out there.
 
 
 Diffs
 -
 
   /branches/12/res/stasis/control.c 420938 
   /branches/12/res/res_stasis.c 420938 
 
 Diff: https://reviewboard.asterisk.org/r/3908/diff/
 
 
 Testing
 ---
 
 Made sure the crash that was happening no longer occurs
 
 Tested long queues of playback to check what would happen with the additional 
 playbacks.  Each subsequent playback from the one currently running fails (in 
 an expected manner) and reports its failure over stasis.  Nothing too odd 
 there.
 
 
 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

Re: [asterisk-dev] [Code Review] 3919: func_config: Change 'Not Found' message from ERROR to DEBUG

2014-08-18 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3919/#review13105
---


Should be done for v1.8+.  That message is kind of over zealous being an ERROR. 
 We recently got rid of a similar over zealous WARNING message in 
URIENCODE/URIDECODE (See ASTERISK-23911).


branches/12/funcs/func_config.c
https://reviewboard.asterisk.org/r/3919/#comment23554

Make this ast_debug(1, ...



- rmudgett


On Aug. 18, 2014, 11:46 a.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3919/
 ---
 
 (Updated Aug. 18, 2014, 11:46 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This one's been bugging me forever...
 
 When you call the CONFIG dialplan function with the name of a variable that 
 doesn't exist in the target context you get an ERROR.  This does nothing but 
 clutter up the logs with messages that may be perfectly acceptable.  Just 
 because a variable wasn't in the context doesn't mean it's an error.  Maybe 
 it's optional or just needs to be defaulted or ignored.
 
 This patch changes the log level from ERROR to DEBUG.  If a dialplan 
 developer wants to debug their dialplan they still can.
 
 
 Diffs
 -
 
   branches/12/funcs/func_config.c 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3919/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 George Joseph
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3920: Fix after bridge behavior when channel moves from a Stasis bridge to a non-Stasis bridge

2014-08-18 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3920/#review13106
---



/branches/13/res/stasis/control.c
https://reviewboard.asterisk.org/r/3920/#comment23555

Could be simplified a bit:

int hangup_flag;
hangup_flag = ast_bridge_setup_after_goto(chan) ? AST_SOFTHANGUP_DEV : 
AST_SOFTHANGUP_ASYNCGOTO;
ast_channel_lock(chan)
ast_softhangup_nolock(chan, hangup_flag)
ast_channel_unlock(chan)


- rmudgett


On Aug. 18, 2014, 1:23 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3920/
 ---
 
 (Updated Aug. 18, 2014, 1:23 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a channel is moved from a Stasis bridge to a non-Stasis bridge, the 
 behavior after the non-Stasis bridge dissolves is incorrect. The issue is 
 that since channels are imparted into Stasis bridges as departable rather 
 than independent, control of the channel returns to the main Stasis 
 application loop after the non-Stasis bridge dissolves. From the end-user 
 perspective, this is most odd.
 
 As an example, say that Alice calls into Stasis and is placed into a Stasis 
 bridge. Bob places a call into the dialplan and calls Bridge(Alice,x), 
 requesting to be bridged with Alice and requesting that Alice be hung up if 
 Bob hangs up first. Alice is pulled from the Stasis, is sent a StasisEnd 
 event, and is pushed into the non-Stasis bridge created by the Bridge 
 application. If Bob were to hang up, the expectation would be that Alice's 
 channel would be hung up as well. However, in actuality, Alice remains in the 
 Stasis application and must be hung up manually. Expecations of Alice's 
 post-bridge destination are also not met when passing the 'F' option or no 
 options at all to the Bridge application.
 
 This patch aims to correct the unexpected behavior by detecting the 
 circumstances when Alice's channel leaves the bridging system. If Alice has 
 already had a StasisEnd published when leaving the bridging system, then 
 Stasis's after bridge callback will attempt to direct Alice to where she is 
 intended to go.
 
 To be honest, I'm not a huge fan of this patch, but it gets the job done. The 
 proper way to fix the issue is to devise a method such that channels that 
 enter Stasis bridges are not departable. However, such a change is of larger 
 scope and risk than is acceptable for Asterisk 12 or 13 (in my judgment 
 anyway), so I have gone with the patch seen here. For Asterisk 14, I would 
 recommend a mini-project to make channels in Stasis bridges independent so 
 that the correct behavior is taken care of innately by the bridging system 
 instead.
 
 
 Diffs
 -
 
   /branches/13/res/stasis/control.c 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3920/diff/
 
 
 Testing
 ---
 
 I created a small ARI application that places any channel that enters the app 
 into a Stasis bridge. I then had a second channel call an extension in the 
 dialplan that called the Bridge application to move the original channel into 
 a non-Stasis bridge. I tested with several options passed to the Bridge 
 application. With the patch, the following occurred:
 
 Bridge(Alice): Channel re-entered Stasis when the non-Stasis bridge dissolved.
 Bridge(Alice,F): Channel moved to the priority after the Bridge() application 
 when the non-Stasis bridge dissolved.
 Bridge(Alice,x): Channel was hung up after the non-Stasis bridge dissolved.
 
 
 Thanks,
 
 Mark Michelson
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3919: func_config: Change 'Not Found' message from ERROR to DEBUG

2014-08-18 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3919/#review13107
---

Ship it!


Ship It!

- rmudgett


On Aug. 18, 2014, 1:47 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3919/
 ---
 
 (Updated Aug. 18, 2014, 1:47 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This one's been bugging me forever...
 
 When you call the CONFIG dialplan function with the name of a variable that 
 doesn't exist in the target context you get an ERROR.  This does nothing but 
 clutter up the logs with messages that may be perfectly acceptable.  Just 
 because a variable wasn't in the context doesn't mean it's an error.  Maybe 
 it's optional or just needs to be defaulted or ignored.
 
 This patch changes the log level from ERROR to DEBUG.  If a dialplan 
 developer wants to debug their dialplan they still can.
 
 
 Diffs
 -
 
   branches/12/funcs/func_config.c 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3919/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 George Joseph
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3921: Stasis: Add missing information to blind transfer events

2014-08-18 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3921/#review13108
---



branches/12/main/bridge.c
https://reviewboard.asterisk.org/r/3921/#comment23558

Blank line between variable declarations and code makes it easier to see 
when the declarations end.



branches/12/main/bridge.c
https://reviewboard.asterisk.org/r/3921/#comment23557

The transferee is partying?



branches/12/main/stasis_bridges.c
https://reviewboard.asterisk.org/r/3921/#comment23556

Please make this one declaration per line.

json_transferer is leaked (new and pre-existing leak)

json_transferee is leaked (new)


- rmudgett


On Aug. 18, 2014, 1:55 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3921/
 ---
 
 (Updated Aug. 18, 2014, 1:55 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a blind transfer occurs that is forced to create a local channel pair to 
 satisfy the transfer request, information about the local channel pair is not 
 published. This adds a field to describe that channel to the blind transfer 
 message struct so that this information is conveyed properly to consumers of 
 the blind transfer message.
 
 This also fixes a bug in which Stasis() was unable to properly identify the 
 channel that was replacing an existing Stasis-controlled channel due to a 
 blind transfer.
 
 
 Diffs
 -
 
   branches/12/rest-api/api-docs/events.json 421326 
   branches/12/res/stasis/app.c 421326 
   branches/12/res/ari/ari_model_validators.c 421326 
   branches/12/res/ari/ari_model_validators.h 421326 
   branches/12/main/stasis_bridges.c 421326 
   branches/12/main/bridge.c 421326 
   branches/12/include/asterisk/stasis_bridges.h 421326 
 
 Diff: https://reviewboard.asterisk.org/r/3921/diff/
 
 
 Testing
 ---
 
 Verified functionality manually and updated the ARI blind transfer test to 
 check for these conditions.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3915: AMI: Add AllVariables parameter to Status

2014-08-18 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3915/#review13109
---



trunk/main/manager.c
https://reviewboard.asterisk.org/r/3915/#comment23559

This could stand to be increased to 128 as an allow=all can create a string 
longer than 64.



trunk/main/manager.c
https://reviewboard.asterisk.org/r/3915/#comment23561

blank line between variable declaration and cocde for clarity.



trunk/main/manager.c
https://reviewboard.asterisk.org/r/3915/#comment23560

ast_channel_connected_effective_id() is a somewhat more expensive operation 
than ast_channel_connected().  It would be best if the party_id were saved to a 
local copy rather than repeatedly called.


- rmudgett


On Aug. 15, 2014, 9:59 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3915/
 ---
 
 (Updated Aug. 15, 2014, 9:59 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds the AllVariables parameter to the Status AMI action such that if 
 defined and set to true, all channel variables will be reported in the 
 subsequent Status event(s). This parameter does not negate the functionality 
 of the Variables parameter so that global variables and dialplan functions 
 can be requested.
 
 
 Diffs
 -
 
   trunk/main/manager.c 420950 
 
 Diff: https://reviewboard.asterisk.org/r/3915/diff/
 
 
 Testing
 ---
 
 Manual testing via netcat and an automated test here: 
 https://reviewboard.asterisk.org/r/3916/
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3904: Replace some code with ao2_replace().

2014-08-14 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3904/
---

(Updated Aug. 14, 2014, 10:54 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 420992


Repository: Asterisk


Description
---

Use ao2_replace() instead of ao2_cleanup(); ao2_bump().
ao2_replace() has the advantange of not altering the ref count if the replaced 
pointer is the same.


Diffs
-

  /branches/13/main/channel_internal_api.c 420838 
  /branches/13/main/channel.c 420838 

Diff: https://reviewboard.asterisk.org/r/3904/diff/


Testing
---

Compiler is still happy.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3905: ARI: Originate to app local channel subscription code optimization.

2014-08-14 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3905/
---

(Updated Aug. 14, 2014, 11:30 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 421009


Repository: Asterisk


Description
---

Reduce the scope of local_peer and only get it if the ARI originate is 
subscribing to the channels.


Diffs
-

  /branches/12/res/ari/resource_channels.c 420855 

Diff: https://reviewboard.asterisk.org/r/3905/diff/


Testing
---

ARI originate of a local channel still goes out.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3910: Fix test failures introduced by 420934

2014-08-14 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3910/#review13086
---

Ship it!


Ship It!

- rmudgett


On Aug. 14, 2014, 1:28 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3910/
 ---
 
 (Updated Aug. 14, 2014, 1:28 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and rmudgett.
 
 
 Bugs: ASTERISK-24027
 https://issues.asterisk.org/jira/browse/ASTERISK-24027
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 r420934 changed ast_channel_is_leaving bridge to not care about the unbridge 
 flag (which is no longer a softhangup flag due to that causing the behavior 
 noted in ASTERISK-24027). This patch makes it so that the unbridge flag is 
 evaluated again since that caused an unexpected change in how media would be 
 re-established when a bridge starts to get torn down on hangup.
 
 
 Diffs
 -
 
   /branches/12/main/channel.c 420934 
 
 Diff: https://reviewboard.asterisk.org/r/3910/diff/
 
 
 Testing
 ---
 
 Ran the failing tests from 
 https://bamboo.asterisk.org/bamboo/browse/AST-ATSF-C632TE-336.  I couldn't 
 get all of them to pass prior to r420934, but the ones that I could run 
 successfully were confirmed broken by r420934 and fixed by this patch.
 
 
 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

[asterisk-dev] [Code Review] 3913: chan_pjsip: Attended transfer does not update connected line name.

2014-08-14 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3913/
---

Review request for Asterisk Developers.


Bugs: AFS-98
https://issues.asterisk.org/jira/browse/AFS-98


Repository: Asterisk


Description
---

A calls B
B SIP attended transfers to C
C answers, B and C can see each other's connected line information
B completes the transfer
A has number but no name connected line information about C while C has the 
full information about A

I examined the incoming and outgoing party id information handling of 
chan_pjsip and found several issues:

* Fixed ast_sip_session_create_outgoing() not setting up the configured 
endpoint id as the new channel's caller id.  This is why party A got default 
connected line information.

* Made update_initial_connected_line() use the channel's CALLERID(id) 
information.  The core, app_dial, or predial routine may have filled in or 
changed the endpoint caller id information.

* Fixed chan_pjsip_new() not setting the full party id information available on 
the caller id and ANI party id.  This includes the configured callerid_tag 
string and other party id fields.

* Fixed accessing channel party id information without the channel lock held.

* Fixed using the effective connected line id without doing a deep copy outside 
of holding the channel lock.  Shallow copy string pointers can become stale if 
the channel lock is not held.

* Made queue_connected_line_update() also update the channel's CALLERID(id) 
information.  Moving the channel to another bridge would need the information 
there for the new bridge peer.

* Fixed off nominal memory leak in update_incoming_connected_line().

* Added callerid_tag string to party id information from enabled trust_inbound 
endpoint in caller_id_incoming_request().


Diffs
-

  /branches/13/res/res_pjsip_session.c 421122 
  /branches/13/res/res_pjsip_caller_id.c 421122 
  /branches/13/channels/chan_pjsip.c 421122 

Diff: https://reviewboard.asterisk.org/r/3913/diff/


Testing
---

Attended transfer gives correct party id information to all parties involved.
Blind transfer gives correct party id information to all parties involved.


Thanks,

rmudgett

-- 
_
-- 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

[asterisk-dev] [Code Review] 3906: chan_pjsip: Update media translation paths when new SDP negotiated.

2014-08-13 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3906/
---

Review request for Asterisk Developers.


Bugs: AFS-137
https://issues.asterisk.org/jira/browse/AFS-137


Repository: Asterisk


Description
---

On a SIP reinvite that changes media strams, the PJSIP channel driver was 
flooding the log with Asked to transmit frame type %s, while native formats is 
%s warnings.

* Fixes PJSIP not setting up translation paths when the formats change on a 
reinvite.

* Improved the unexpected frame format WARNING message to include more 
information.

* Added protective locking while altering formats on a channel.  Reworked 
set_format() to simplify and protect the formats under manipulation.

* Restored some code that got lost in the media_formats work.


Diffs
-

  /branches/13/res/res_pjsip_sdp_rtp.c 420978 
  /branches/13/main/file.c 420978 
  /branches/13/main/channel.c 420978 
  /branches/13/main/bridge_channel.c 420978 
  /branches/13/main/bridge.c 420978 
  /branches/13/channels/chan_pjsip.c 420978 

Diff: https://reviewboard.asterisk.org/r/3906/diff/


Testing
---

Performed blind SIP transfers (REFER) and the call still passes media.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3903: Stasis: Allow internal channels directly into bridges

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3903/#review13075
---



branches/12/res/res_stasis.c
https://reviewboard.asterisk.org/r/3903/#comment23514

This variable is effectively unused.  Nothing seems to care about the value 
set in res.  That is unless you actually intend to return the value but forgot 
to.


- rmudgett


On Aug. 11, 2014, 1:35 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3903/
 ---
 
 (Updated Aug. 11, 2014, 1:35 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The patch to catch channels being shoehorned into Stasis() via external 
 mechanisms also happens to catch Announcer and Recorder channels because they 
 aren't known to be stasis-controlled channels in the usual sense. This marks 
 those channels as Stasis()-internal channels and allows them directly into 
 bridges.
 
 
 Diffs
 -
 
   branches/12/res/stasis/stasis_bridge.c 420591 
   branches/12/res/res_stasis.c 420591 
   branches/12/res/ari/resource_bridges.c 420591 
   branches/12/include/asterisk/stasis_app.h 420591 
 
 Diff: https://reviewboard.asterisk.org/r/3903/diff/
 
 
 Testing
 ---
 
 Ensured this patch fixed rest_api/bridges/playback/tones and 
 rest_api/bridges/bridge_record tests.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3900: Bridging: Fix an issue where bridge features can be interrupted by actions that set the UNBRIDGE soft hangup flag on a channel.

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3900/#review13076
---



/branches/12/apps/app_chanspy.c
https://reviewboard.asterisk.org/r/3900/#comment23516

use nolock version



/branches/12/apps/app_mixmonitor.c
https://reviewboard.asterisk.org/r/3900/#comment23517

idem



/branches/12/apps/app_stack.c
https://reviewboard.asterisk.org/r/3900/#comment23515

Extra parens.  This line may be short enought to not need line continuation.



/branches/12/main/channel_internal_api.c
https://reviewboard.asterisk.org/r/3900/#comment23518

Why were these changed?  They were already using _nolock suffix.
ast_channel_set_unbridged() and ast_channel_set_unbridged_nolock()


- rmudgett


On Aug. 11, 2014, 1:23 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3900/
 ---
 
 (Updated Aug. 11, 2014, 1:23 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and rmudgett.
 
 
 Bugs: ASTERISK-24027
 https://issues.asterisk.org/jira/browse/ASTERISK-24027
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 An odd little issue that can interrupt features and even hangup calls 
 entirely in an unwanted fashion.
 
 Reproduction is fairly trivial, simply have a dynamic bridge feature that 
 plays audio or executes an AGI or something that just generally takes a 
 little while and is sensitive to hangups.
 Call into an extension that puts that feature on the channel and call another 
 device.
 Execute the feature.
 While the feature is running, start mixmonitor on the channel executing the 
 feature and watch as the feature is prematurely terminated and the call 
 itself is hung up entirely.
 
 Having the flag to re-evaluate the status of the bridge be a hangup flag 
 seems to have been a little off point and trying to have everything that pays 
 attention to hangups specifically ignore it seems a little wacky, so instead 
 I've pulled AST_SOFTHANGUP_UNBRIDGE out of the soft hangup flags and turned 
 it into its own thing.
 
 
 Diffs
 -
 
   /branches/12/main/pbx.c 420558 
   /branches/12/main/framehook.c 420558 
   /branches/12/main/channel_internal_api.c 420558 
   /branches/12/main/channel.c 420558 
   /branches/12/main/bridge_channel.c 420558 
   /branches/12/main/bridge_after.c 420558 
   /branches/12/include/asterisk/channel.h 420558 
   /branches/12/apps/app_stack.c 420558 
   /branches/12/apps/app_mixmonitor.c 420558 
   /branches/12/apps/app_chanspy.c 420558 
 
 Diff: https://reviewboard.asterisk.org/r/3900/diff/
 
 
 Testing
 ---
 
 Performed the above reproduction steps with the patch and the call no longer 
 hangs up and the feature completes normally.  Mixmonitor captures all the 
 audio as well.
 Made sure native RTP bridges would still be re-evaluated and become simple 
 bridges when a hook such as mixmonitor is placed on one of the bridged 
 channels.
 Ran through chan_sip and chan_pjsip testsuite tests to make sure the patch 
 didn't introduce any failures.
 
 
 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

Re: [asterisk-dev] [Code Review] 3900: Bridging: Fix an issue where bridge features can be interrupted by actions that set the UNBRIDGE soft hangup flag on a channel.

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3900/#review13077
---



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3900/#comment23519

Remove unbridged references.  Also there are extra parens around the 
ASYNCGOTO flag.


- rmudgett


On Aug. 11, 2014, 1:23 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3900/
 ---
 
 (Updated Aug. 11, 2014, 1:23 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and rmudgett.
 
 
 Bugs: ASTERISK-24027
 https://issues.asterisk.org/jira/browse/ASTERISK-24027
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 An odd little issue that can interrupt features and even hangup calls 
 entirely in an unwanted fashion.
 
 Reproduction is fairly trivial, simply have a dynamic bridge feature that 
 plays audio or executes an AGI or something that just generally takes a 
 little while and is sensitive to hangups.
 Call into an extension that puts that feature on the channel and call another 
 device.
 Execute the feature.
 While the feature is running, start mixmonitor on the channel executing the 
 feature and watch as the feature is prematurely terminated and the call 
 itself is hung up entirely.
 
 Having the flag to re-evaluate the status of the bridge be a hangup flag 
 seems to have been a little off point and trying to have everything that pays 
 attention to hangups specifically ignore it seems a little wacky, so instead 
 I've pulled AST_SOFTHANGUP_UNBRIDGE out of the soft hangup flags and turned 
 it into its own thing.
 
 
 Diffs
 -
 
   /branches/12/main/pbx.c 420558 
   /branches/12/main/framehook.c 420558 
   /branches/12/main/channel_internal_api.c 420558 
   /branches/12/main/channel.c 420558 
   /branches/12/main/bridge_channel.c 420558 
   /branches/12/main/bridge_after.c 420558 
   /branches/12/include/asterisk/channel.h 420558 
   /branches/12/apps/app_stack.c 420558 
   /branches/12/apps/app_mixmonitor.c 420558 
   /branches/12/apps/app_chanspy.c 420558 
 
 Diff: https://reviewboard.asterisk.org/r/3900/diff/
 
 
 Testing
 ---
 
 Performed the above reproduction steps with the patch and the call no longer 
 hangs up and the feature completes normally.  Mixmonitor captures all the 
 audio as well.
 Made sure native RTP bridges would still be re-evaluated and become simple 
 bridges when a hook such as mixmonitor is placed on one of the bridged 
 channels.
 Ran through chan_sip and chan_pjsip testsuite tests to make sure the patch 
 didn't introduce any failures.
 
 
 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

Re: [asterisk-dev] [Code Review] 3900: Bridging: Fix an issue where bridge features can be interrupted by actions that set the UNBRIDGE soft hangup flag on a channel.

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3900/#review13080
---



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3900/#comment23522

double parens ((...))



/branches/12/main/framehook.c
https://reviewboard.asterisk.org/r/3900/#comment23523

_nolock version



/branches/12/main/framehook.c
https://reviewboard.asterisk.org/r/3900/#comment23524

_nolock version


- rmudgett


On Aug. 11, 2014, 4:15 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3900/
 ---
 
 (Updated Aug. 11, 2014, 4:15 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and rmudgett.
 
 
 Bugs: ASTERISK-24027
 https://issues.asterisk.org/jira/browse/ASTERISK-24027
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 An odd little issue that can interrupt features and even hangup calls 
 entirely in an unwanted fashion.
 
 Reproduction is fairly trivial, simply have a dynamic bridge feature that 
 plays audio or executes an AGI or something that just generally takes a 
 little while and is sensitive to hangups.
 Call into an extension that puts that feature on the channel and call another 
 device.
 Execute the feature.
 While the feature is running, start mixmonitor on the channel executing the 
 feature and watch as the feature is prematurely terminated and the call 
 itself is hung up entirely.
 
 Having the flag to re-evaluate the status of the bridge be a hangup flag 
 seems to have been a little off point and trying to have everything that pays 
 attention to hangups specifically ignore it seems a little wacky, so instead 
 I've pulled AST_SOFTHANGUP_UNBRIDGE out of the soft hangup flags and turned 
 it into its own thing.
 
 
 Diffs
 -
 
   /branches/12/main/pbx.c 420558 
   /branches/12/main/framehook.c 420558 
   /branches/12/main/channel_internal_api.c 420558 
   /branches/12/main/channel.c 420558 
   /branches/12/main/bridge_channel.c 420558 
   /branches/12/main/bridge_after.c 420558 
   /branches/12/include/asterisk/channel.h 420558 
   /branches/12/apps/app_stack.c 420558 
   /branches/12/apps/app_mixmonitor.c 420558 
   /branches/12/apps/app_chanspy.c 420558 
 
 Diff: https://reviewboard.asterisk.org/r/3900/diff/
 
 
 Testing
 ---
 
 Performed the above reproduction steps with the patch and the call no longer 
 hangs up and the feature completes normally.  Mixmonitor captures all the 
 audio as well.
 Made sure native RTP bridges would still be re-evaluated and become simple 
 bridges when a hook such as mixmonitor is placed on one of the bridged 
 channels.
 Ran through chan_sip and chan_pjsip testsuite tests to make sure the patch 
 didn't introduce any failures.
 
 
 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

[asterisk-dev] [Code Review] 3904: Replace some code with ao2_replace().

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3904/
---

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Use ao2_replace() instead of ao2_cleanup(); ao2_bump().
ao2_replace() has the advantange of not altering the ref count if the replaced 
pointer is the same.


Diffs
-

  /branches/13/main/channel_internal_api.c 420838 
  /branches/13/main/channel.c 420838 

Diff: https://reviewboard.asterisk.org/r/3904/diff/


Testing
---

Compiler is still happy.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3900: Bridging: Fix an issue where bridge features can be interrupted by actions that set the UNBRIDGE soft hangup flag on a channel.

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3900/#review13081
---

Ship it!


Ship It!

- rmudgett


On Aug. 11, 2014, 5:06 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3900/
 ---
 
 (Updated Aug. 11, 2014, 5:06 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and rmudgett.
 
 
 Bugs: ASTERISK-24027
 https://issues.asterisk.org/jira/browse/ASTERISK-24027
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 An odd little issue that can interrupt features and even hangup calls 
 entirely in an unwanted fashion.
 
 Reproduction is fairly trivial, simply have a dynamic bridge feature that 
 plays audio or executes an AGI or something that just generally takes a 
 little while and is sensitive to hangups.
 Call into an extension that puts that feature on the channel and call another 
 device.
 Execute the feature.
 While the feature is running, start mixmonitor on the channel executing the 
 feature and watch as the feature is prematurely terminated and the call 
 itself is hung up entirely.
 
 Having the flag to re-evaluate the status of the bridge be a hangup flag 
 seems to have been a little off point and trying to have everything that pays 
 attention to hangups specifically ignore it seems a little wacky, so instead 
 I've pulled AST_SOFTHANGUP_UNBRIDGE out of the soft hangup flags and turned 
 it into its own thing.
 
 
 Diffs
 -
 
   /branches/12/main/pbx.c 420558 
   /branches/12/main/framehook.c 420558 
   /branches/12/main/channel_internal_api.c 420558 
   /branches/12/main/channel.c 420558 
   /branches/12/main/bridge_channel.c 420558 
   /branches/12/main/bridge_after.c 420558 
   /branches/12/include/asterisk/channel.h 420558 
   /branches/12/apps/app_stack.c 420558 
   /branches/12/apps/app_mixmonitor.c 420558 
   /branches/12/apps/app_chanspy.c 420558 
 
 Diff: https://reviewboard.asterisk.org/r/3900/diff/
 
 
 Testing
 ---
 
 Performed the above reproduction steps with the patch and the call no longer 
 hangs up and the feature completes normally.  Mixmonitor captures all the 
 audio as well.
 Made sure native RTP bridges would still be re-evaluated and become simple 
 bridges when a hook such as mixmonitor is placed on one of the bridged 
 channels.
 Ran through chan_sip and chan_pjsip testsuite tests to make sure the patch 
 didn't introduce any failures.
 
 
 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

[asterisk-dev] [Code Review] 3905: ARI: Originate to app local channel subscription code optimization.

2014-08-11 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3905/
---

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Reduce the scope of local_peer and only get it if the ARI originate is 
subscribing to the channels.


Diffs
-

  /branches/12/res/ari/resource_channels.c 420855 

Diff: https://reviewboard.asterisk.org/r/3905/diff/


Testing
---

ARI originate of a local channel still goes out.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3833: Allow sending voicemail to multiple email addresses

2014-08-09 Thread rmudgett 


 On Aug. 9, 2014, 3:42 p.m., abelbeck wrote:
  trunk/apps/app_voicemail.c, line 5045
  https://reviewboard.asterisk.org/r/3833/diff/2/?file=66291#file66291line5045
 
  next appears to be off by one, not generating enough commas.
  
  char *next = emailsbuf;
  
  fixes the issue for me.

Please file an issue on the issue tracker as this patch has been commited.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3833/#review13058
---


On Aug. 8, 2014, 2:15 p.m., Jason Parker wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3833/
 ---
 
 (Updated Aug. 8, 2014, 2:15 p.m.)
 
 
 Review request for Asterisk Developers and Jacob Barber.
 
 
 Bugs: ASTERISK-24045
 https://issues.asterisk.org/jira/browse/ASTERISK-24045
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Allow sending voicemail to multiple email addresses.
 
 Should this supersede https://reviewboard.asterisk.org/r/3829/ ?  I would 
 think so.
 
 
 Diffs
 -
 
   trunk/configs/samples/voicemail.conf.sample 420313 
   trunk/apps/app_voicemail.c 420313 
 
 Diff: https://reviewboard.asterisk.org/r/3833/diff/
 
 
 Testing
 ---
 
 Patch in FreePBX distro for ~6 months.
 
 
 Thanks,
 
 Jason Parker
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3829: Voicemail send email to multiple email addresses

2014-08-08 Thread rmudgett 


 On July 29, 2014, 9:44 a.m., Matt Jordan wrote:
  Per https://reviewboard.asterisk.org/r/3833/, there is an alternate patch 
  that performs the same functionality as the one proposed here.
  
  Do you have any objection to going with the solution proposed on r3833?

This review should be closed as discarded as the other review has been 
committed.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3829/#review12905
---


On July 18, 2014, 1:35 p.m., Jacob Barber wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3829/
 ---
 
 (Updated July 18, 2014, 1:35 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24045
 https://issues.asterisk.org/jira/browse/ASTERISK-24045
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Currently voicemail to email only works with a single email. This patch 
 allows a user to use a space separated list of emails (up to 512 characters 
 long), where the user would like for the emails to be sent. This is useful 
 for people who don't want to go through setting up mailing groups, or for 
 people who host provide VoIP services with asterisk as a backend, where their 
 customers don't know how to set up mailing groups.
 
 
 Diffs
 -
 
   /branches/12/apps/app_voicemail.c 418713 
 
 Diff: https://reviewboard.asterisk.org/r/3829/diff/
 
 
 Testing
 ---
 
 Tested calling and sending voicemails using the mysql realtime database and 
 using the standard voicemail.conf implementation.
 
 
 Thanks,
 
 Jacob Barber
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3900: Bridging: Fix an issue where bridge features can be interrupted by actions that set the UNBRIDGE soft hangup flag on a channel.

2014-08-08 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3900/#review13057
---



/branches/12/apps/app_stack.c
https://reviewboard.asterisk.org/r/3900/#comment23489

Should just remove the UNBRIDGE references as this code is only for 
softhangup flags.



/branches/12/apps/app_stack.c
https://reviewboard.asterisk.org/r/3900/#comment23490

Restore this code as ASYNCGOTO is still a thing.



/branches/12/include/asterisk/channel.h
https://reviewboard.asterisk.org/r/3900/#comment23491

Leave a hole in the bit definitions for ABI compatibility.



/branches/12/include/asterisk/channel.h
https://reviewboard.asterisk.org/r/3900/#comment23492

No need to state why the function was created.  Only what it does.



/branches/12/include/asterisk/channel.h
https://reviewboard.asterisk.org/r/3900/#comment23493

This goes against the established defacto pattern.  _locked vs _nolock



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3900/#comment23496

This function may only care about ASYNCGOTO now.  Unbridge may no longer 
have anything to do with this function.



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3900/#comment23494

There is only one non-hangup flag left so the extra parens aren't necessary.



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3900/#comment23495

Unbridge no longer has a part in this.



/branches/12/main/channel_internal_api.c
https://reviewboard.asterisk.org/r/3900/#comment23497

These functions names are opposite from defacto convention: _nolock vs 
_locked



/branches/12/main/pbx.c
https://reviewboard.asterisk.org/r/3900/#comment23498

This can probably just be removed.  It is no longer a softhangup flag.  If 
it does leak out of the bridge framework, it won't cause any harm.  The next 
time the channel goes into the bridging framework, it would just get 
reevaluated unnecessarily.

You could instead always clear the unbridge flag when a channel joins the 
bridging system.


- rmudgett


On Aug. 8, 2014, 5:42 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3900/
 ---
 
 (Updated Aug. 8, 2014, 5:42 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and rmudgett.
 
 
 Bugs: ASTERISK-24027
 https://issues.asterisk.org/jira/browse/ASTERISK-24027
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 An odd little issue that can interrupt features and even hangup calls 
 entirely in an unwanted fashion.
 
 Reproduction is fairly trivial, simply have a dynamic bridge feature that 
 plays audio or executes an AGI or something that just generally takes a 
 little while and is sensitive to hangups.
 Call into an extension that puts that feature on the channel and call another 
 device.
 Execute the feature.
 While the feature is running, start mixmonitor on the channel executing the 
 feature and watch as the feature is prematurely terminated and the call 
 itself is hung up entirely.
 
 Having the flag to re-evaluate the status of the bridge be a hangup flag 
 seems to have been a little off point and trying to have everything that pays 
 attention to hangups specifically ignore it seems a little wacky, so instead 
 I've pulled AST_SOFTHANGUP_UNBRIDGE out of the soft hangup flags and turned 
 it into its own thing.
 
 
 Diffs
 -
 
   /branches/12/main/pbx.c 420558 
   /branches/12/main/framehook.c 420558 
   /branches/12/main/channel_internal_api.c 420558 
   /branches/12/main/channel.c 420558 
   /branches/12/main/bridge_channel.c 420558 
   /branches/12/main/bridge_after.c 420558 
   /branches/12/include/asterisk/channel.h 420558 
   /branches/12/apps/app_stack.c 420558 
   /branches/12/apps/app_mixmonitor.c 420558 
   /branches/12/apps/app_chanspy.c 420558 
 
 Diff: https://reviewboard.asterisk.org/r/3900/diff/
 
 
 Testing
 ---
 
 Performed the above reproduction steps with the patch and the call no longer 
 hangs up and the feature completes normally.  Mixmonitor captures all the 
 audio as well.
 Made sure native RTP bridges would still be re-evaluated and become simple 
 bridges when a hook such as mixmonitor is placed on one of the bridged 
 channels.
 Ran through chan_sip and chan_pjsip testsuite tests to make sure the patch 
 didn't introduce any failures.
 
 
 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

Re: [asterisk-dev] [Code Review] 3723: RLS: NOTIFY generation + structural refactor

2014-08-07 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3723/#review13049
---

Ship it!


Ship It!

- rmudgett


On Aug. 7, 2014, 9:23 a.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3723/
 ---
 
 (Updated Aug. 7, 2014, 9:23 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23869
 https://issues.asterisk.org/jira/browse/ASTERISK-23869
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds NOTIFY support for resource list subscriptions. The way this works 
 is as follows:
 
 When an initial SUBSCRIBE arrives and the subscription tree is built, all 
 leaf nodes are called into in order to generate their initial NOTIFY bodies 
 and store these on their respective subscription nodes.
 
 Sending a NOTIFY requires traversing the tree. List subscriptions will 
 generate a multipart/related body with an RLMI part and parts corresponding 
 to the leaves of the list (at least they will eventually. ASTERISK-23867 
 involves doing this part). Single-resource subscriptions read the stored body 
 on the subscription and uses that to populate the NOTIFY body.
 
 Leaf nodes in the subscription tree, when they have a state change occur, 
 call ast_sip_subscription_notify(), as they previously did. 
 ast_sip_subscription_notify() creates the NOTIFY body for the subscription 
 and stores that on the subscription in the body_text ast_str field. The 
 subscription tree is then told to send a NOTIFY if no batching is enabled or 
 to start a batched NOTIFY if batching is enabled.
 
 When a resource resubscribes or terminates their subscription, a NOTIFY is 
 now automatically generated by the pubsub core instead of calling into 
 subscription handlers. The NOTIFY is built the same way as previously, using 
 stored NOTIFY bodies on the subscription. This NOTIFY can also cause batched 
 notification, when the timer fires, not to actually send their batch since it 
 would be redundant.
 
 You'll notice the code has been refactored slightly, and a new struct, 
 sip_subscription_tree, has invaded res_pjsip_pubsub. This is because, as I 
 was separating the real and virtual parts of ast_sip_subscriptions out, I 
 realized that I essentially had two distinct structures. Thus, I separated 
 the real/meta/base elements of a subscription into the sip_subscription_tree, 
 and the resource-specific parts into the ast_sip_subscription struct. 
 sip_subscription_tree is used more heavily in the pubsub core now, whereas 
 ast_sip_subscription acts as a handle for subscription handlers to use plus a 
 repository for resource-specific data.
 
 
 Diffs
 -
 
   /team/group/rls/res/res_pjsip_pubsub.c 420264 
   /team/group/rls/res/res_pjsip_mwi.c 420264 
   /team/group/rls/res/res_pjsip_exten_state.c 420264 
   /team/group/rls/include/asterisk/res_pjsip_pubsub.h 420264 
 
 Diff: https://reviewboard.asterisk.org/r/3723/diff/
 
 
 Testing
 ---
 
 With this set of changes, I'm still not able to perform RLS-specific tests 
 since there is still no method of generating multipart/related or RLMI 
 bodies. However, with these changes, I did run the gamut of subscription 
 tests in the testsuite and they all pass. This at least means that there are 
 no detectable regressions at this point.
 
 
 Thanks,
 
 Mark Michelson
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3890: chan_iax2: Several media format fixes.

2014-08-07 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3890/
---

(Updated Aug. 7, 2014, 1:51 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 420364


Bugs: ASTERISK-24150
https://issues.asterisk.org/jira/browse/ASTERISK-24150


Repository: Asterisk


Description
---

* Fixed the iax.conf bandwidth option.  This is the root cause of
ASTERISK-24150.

* Added checks in iax2_request() to ensure that there are actual formats
requested for the new channel to prevent any more fracks from issues like
ASTERISK-24150.  This is a consequence of the iax.conf bandwidth option
not working.

* Fixed struct iax2_codec_pref.order member size mismatch issue when
converting to and from the codec preference order list passed over the
wire.  In addition the values sent over the wire are now compatible with
previous Asterisk versions.

* Fixed several issues dealing with the struct iax2_codec_pref members.
Off-by-one, array limit errors, and the order/framing members always need
to be updated together.

* Made iax2_request() setup the channel's native format preference order
according to the user's wishes.  The new media format strategy needs the
order specified earler.

* Fixed usage of ast_format_compatibility_bitfield2format().  The function
can return NULL if the bitfield was not associated with a function.

* Deleted dead code iax2_codec_pref_getsize() and
iax2_codec_pref_setsize().

* Made iax2_parse_allow_disallow() and iax2_codec_pref_string() call
iax2_codec_pref_to_cap() instead of inlining it.

* Made IAX_CAPABILITY_MEDBANDWIDTH, IAX_CAPABILITY_LOWBANDWIDTH, and
IAX_CAPABILITY_LOWFREE constants again as they were in Asterisk v1.8.

* Renamed prefs to prefs_global so it won't get confused with the local
pref versions.

* Fixed too small buffer in handle_cli_iax2_show_peer().

* Fixed ast_cli() calls in handle_cli_iax2_show_peer() to output complete
lines.

* Changed struct create_addr_info.prefs to be struct iax2_codec_pref as an
optimization so iax2_request() and iax2_call() do less work.

* Fixed a potential deadlock in ast_iax2_new() on an off-nominal path when
the pbx could not get started.

* Made set_config() setup a local prefs list along side the local
capability format bitfield.  Once the config is loaded, then the local
copies are put into the global versions.

* Fix unininialized codec_buf in function_iaxpeer().


Diffs
-

  /trunk/main/format_compatibility.c 420087 
  /trunk/include/asterisk/format_compatibility.h 420087 
  /trunk/channels/iax2/include/format_compatibility.h 420087 
  /trunk/channels/iax2/include/codec_pref.h 420087 
  /trunk/channels/iax2/format_compatibility.c 420087 
  /trunk/channels/iax2/codec_pref.c 420087 
  /trunk/channels/chan_iax2.c 420087 

Diff: https://reviewboard.asterisk.org/r/3890/diff/


Testing
---

* The 12 testsuite tests tagged with iax2 pass.

* The iax.conf bandwidth option is now functional and no longer causes a frack 
when a call is made.

* The CLI iax2 show peer has enough buffer space to generate a longer codec 
list for the configured codecs header.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3653: chan_sip: (Optionally) poll even on first part of TLS message

2014-08-07 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3653/#review13054
---


Please close this review as discarded.  The other review has been committed.

- rmudgett


On June 20, 2014, 9:06 a.m., Alexander Traud wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3653/
 ---
 
 (Updated June 20, 2014, 9:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With some large SDP, a *second* poll is required on the first part of a TLS 
 message.
 
 The current code did not poll a second time because the variable need_poll 
 was inited with yes (1). That poll was a no-operation because there was a 
 socket event already (which mandates fgets without poll). In the current 
 code, poll returned immediately, fgets returned NULL, after_poll was yes (1), 
 sip_tls_read returned failed (-1), _sip_tcp_helper_thread went to cleanup, 
 called ast_tcptls_close_session_file, which closed the TLS connection.
 
 The proposed patch, reads the gets the first message. If that failed, it does 
 poll. This fixed all large SDP issues with SIP over TLS which I faced.
 
 I am aware there were changes committed to tcptls.c just recently (revision 
 415907). Anyway, let us fix this bug as well.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 416319 
 
 Diff: https://reviewboard.asterisk.org/r/3653/diff/
 
 
 Testing
 ---
 
 Asterisk 12.3
 
 
 Thanks,
 
 Alexander Traud
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3607: [app_queue] Add the optional ability to load queue rules from realtime.

2014-08-07 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3607/#review13055
---


This patch has no documentation describing how to use the new feature.
Appropriate changes need to be made to the sample config files as well as a 
CHANGES note.

- rmudgett


On July 30, 2014, 10:07 a.m., Michael K. wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3607/
 ---
 
 (Updated July 30, 2014, 10:07 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23823
 https://issues.asterisk.org/jira/browse/ASTERISK-23823
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch gives the ability (optional) to keep queuerules in realtime 
 (important to notice that rules would be loaded from both realtime and file 
 too)
 To use add to queuerules.conf general section with option to turn the 
 relatime on (note that after patch there is no option to call rule general 
 as it is reserved for queuerules settings):
 
 [general]
 realtime_rules = yes
 
 and add the realtime setting to extconfig.conf, for example in my case it's 
 mysql and it looks like(will attach the .sql with create for table):
 
 queue_rules = mysql,asterisk,queue_rules
 
 in table as you can see you need to specify rule name and as in regular rules 
 you can use relative setings for min and max (with - and +, e.g. +100)
 if one of the rule parameters is wrong it would be ignored, basically 
 validation is like from file.
 here is the the example of table context as an example:
 
 rule_name, time, min_penalty, max_penalty
 'default', '10', '20', '30'
 'test2', '20', '30', '55'
 'test2', '25', '-11', '+'
 'test2', '400', '112', '333'
 'test3', '0', '4564', '46546'
 'test_rule', '40', '15', '50'
 
 which would result in :
 
 Rule: default
   After 10 seconds, adjust QUEUE_MAX_PENALTY to 30 and adjust 
 QUEUE_MIN_PENALTY to 20
 Rule: test2
   After 20 seconds, adjust QUEUE_MAX_PENALTY to 55 and adjust 
 QUEUE_MIN_PENALTY to 30
   After 25 seconds, adjust QUEUE_MAX_PENALTY by  and adjust 
 QUEUE_MIN_PENALTY by -11
   After 400 seconds, adjust QUEUE_MAX_PENALTY to 333 and adjust 
 QUEUE_MIN_PENALTY to 112
 Rule: test3
   After 0 seconds, adjust QUEUE_MAX_PENALTY to 46546 and adjust 
 QUEUE_MIN_PENALTY to 4564
 Rule: test_rule
   After 40 seconds, adjust QUEUE_MAX_PENALTY to 50 and adjust 
 QUEUE_MIN_PENALTY to 15
 
 
 If you use realtime, the rules will be always reloaded (no cache option 
 here), in other words it would delete all rules and re-add them from realtime 
 and file. It's important to understand that with realtime on it would reload 
 rules from file doesn't matter if file was or was not change.
 While with the option off it would act exactly like it was before patch (file 
 would not be reloaded if it was not changed)
 
 
 Diffs
 -
 
   trunk/apps/app_queue.c 415442 
 
 Diff: https://reviewboard.asterisk.org/r/3607/diff/
 
 
 Testing
 ---
 
 Currently the tests were made on development machine.
 
 
 Thanks,
 
 Michael K.
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3882: Replace sip_tls_read() and resolve the large SDP poll issue

2014-08-06 Thread rmudgett 


 On Aug. 6, 2014, 1:58 a.m., Alexander Traud wrote:
  trunk/channels/chan_sip.c, line 3042
  https://reviewboard.asterisk.org/r/3882/diff/2/?file=66262#file66262line3042
 
  Because the rest of the Asterisk code does it this way:
  instead of bitwiseOr |
  please, a logicalOr ||
  see 
  http://stackoverflow.com/questions/3154132/what-is-the-difference-between-logical-and-conditional-and-or-in-c
  
  One reason, I would use an else if for the EINTR case. However, I am 
  not sure if the coding-style guides have a rule for this.
  
  @Richard
  I am just curious after reading the code: When is EINTR possible? Or is 
  that just a coding convention?
 
 wdoekes wrote:
 If the call is interrupted by a signal handler, it may return EINTR. 
 Example:
 
   #include errno.h
   #include signal.h
   #include stdio.h
   #include unistd.h
   void handler(int signum) {
 fprintf(stderr, got sigalarm\n);
   }
   int main() {
 char buf[1];
 struct sigaction act = {0,}; // ignore 
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
 
 act.sa_handler = handler;
 act.sa_flags = 0; //SA_RESTART;
 sigaction(SIGALRM, act, NULL);
 //signal(SIGALRM, handler);
 
 alarm(1);
 read(0, buf, 1);
 
 if (errno == EINTR) {
 fprintf(stderr, got EINTR\n);
 }
 return 0;
   }
 
 I'm not sure SA_RESTART is used consistently in asterisk, seeing that both
 signal(2) and sigaction(2) are used. So EINTR might happen on systems that
 do not default to SA_RESTART. In any case, it's convention and smart to
 check for both.
 
 
 As for the code, I'd prefer this, without the extra else block.
 The continue already makes sure that we don't get to the next statements.
 
   if (errno == EAGAIN || errno == EINTR) {
   continue;
   }
   ast_debug(2, SIP TCP/TLS server error when receiving data\n);
   return -1;
 

 
 Alexander Traud wrote:
 Walter, I am about ast_tcptls_server_read() which handles EINTR already. 
 There, I did not find the condition which returns EINTR to us here in this 
 code.

The ast_tcptls_server_read() call will not handle the EINTR in this case 
because the previous call to ast_tcptls_stream_set_exclusive_input() in 
_sip_tcp_helper_thread() tells it not to wait.

Also please change the if statement as walter mentioned.  Use of the bitwise or 
in that manner is unusual.  Where did you see this done elsewhere?


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3882/#review13016
---


On Aug. 5, 2014, 9:21 p.m., ebroad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3882/
 ---
 
 (Updated Aug. 5, 2014, 9:21 p.m.)
 
 
 Review request for Asterisk Developers and Alexander Traud.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Replace sip_tls_read() and sip_tcp_read() with a single function and resolve 
 the poll/wait issue with large SDP payloads. See 
 https://reviewboard.asterisk.org/r/3653/ for the discussion on this.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 419821 
 
 Diff: https://reviewboard.asterisk.org/r/3882/diff/
 
 
 Testing
 ---
 
 Made and received calls successfully with CSipSimple with full SIP headers 
 over TLS, SRTP and multiple codecs enabled ensuring a large SDP payload.
 
 
 Thanks,
 
 ebroad
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3870: alembic: Adjust sippeers, queue_members, and voicemail_messages tables.

2014-08-06 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3870/
---

(Updated Aug. 6, 2014, 11:51 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 420211


Bugs: ASTERISK-23825, ASTERISK-23847 and ASTERISK-23909
https://issues.asterisk.org/jira/browse/ASTERISK-23825
https://issues.asterisk.org/jira/browse/ASTERISK-23847
https://issues.asterisk.org/jira/browse/ASTERISK-23909


Repository: Asterisk


Description
---

* Increased the sippeers useragent max string size to 255.

* Changed the queue_members uniqueid to an auto incremented integer instead of 
a string.

* Increased the voicemail_messages BLOB size to LONGBLOB on mysql.

* Fixed the add_tables_for_pjsip config change version downgrade actions to 
drop a table it created.

* Adjusted the sample alembic.ini files cdr.ini.sample, config.ini.sample, and 
voicemail.ini.sample to give a mysql and postgres sqlalchemy.url lines.


Diffs
-

  
/branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py
 PRE-CREATION 
  /branches/12/contrib/ast-db-manage/voicemail.ini.sample 420024 
  
/branches/12/contrib/ast-db-manage/config/versions/5139253c0423_make_q_member_uniqueid_autoinc.py
 PRE-CREATION 
  
/branches/12/contrib/ast-db-manage/config/versions/43956d550a44_add_tables_for_pjsip.py
 420024 
  
/branches/12/contrib/ast-db-manage/config/versions/1758e8bbf6b_increase_useragent_column_size.py
 PRE-CREATION 
  /branches/12/contrib/ast-db-manage/config.ini.sample 420024 
  /branches/12/contrib/ast-db-manage/cdr.ini.sample 420024 

Diff: https://reviewboard.asterisk.org/r/3870/diff/


Testing
---

The alembic table adjustments work in the upgrade and downgrade modes for mysql 
and postgres.

The queue_members change does give a postgres warning message from the postgres 
alembic backend about a mysql only kind of change.  The warning is benign and 
can be ignored as it is an alembic only warning.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3882: Replace sip_tls_read() and resolve the large SDP poll issue

2014-08-06 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3882/#review13038
---

Ship it!


Ship It!

- rmudgett


On Aug. 6, 2014, 1:04 p.m., ebroad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3882/
 ---
 
 (Updated Aug. 6, 2014, 1:04 p.m.)
 
 
 Review request for Asterisk Developers and Alexander Traud.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Replace sip_tls_read() and sip_tcp_read() with a single function and resolve 
 the poll/wait issue with large SDP payloads. See 
 https://reviewboard.asterisk.org/r/3653/ for the discussion on this.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 419821 
 
 Diff: https://reviewboard.asterisk.org/r/3882/diff/
 
 
 Testing
 ---
 
 Made and received calls successfully with CSipSimple with full SIP headers 
 over TLS, SRTP and multiple codecs enabled ensuring a large SDP payload.
 
 
 Thanks,
 
 ebroad
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3882: Replace sip_tls_read() and resolve the large SDP poll issue

2014-08-06 Thread rmudgett 


 On Aug. 6, 2014, 2:22 p.m., rmudgett wrote:
  Ship It!

I'll commit this tomorrow if you don't have commit access.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3882/#review13038
---


On Aug. 6, 2014, 1:04 p.m., ebroad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3882/
 ---
 
 (Updated Aug. 6, 2014, 1:04 p.m.)
 
 
 Review request for Asterisk Developers and Alexander Traud.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Replace sip_tls_read() and sip_tcp_read() with a single function and resolve 
 the poll/wait issue with large SDP payloads. See 
 https://reviewboard.asterisk.org/r/3653/ for the discussion on this.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 419821 
 
 Diff: https://reviewboard.asterisk.org/r/3882/diff/
 
 
 Testing
 ---
 
 Made and received calls successfully with CSipSimple with full SIP headers 
 over TLS, SRTP and multiple codecs enabled ensuring a large SDP payload.
 
 
 Thanks,
 
 ebroad
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3723: RLS: NOTIFY generation + structural refactor

2014-08-06 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3723/#review13033
---



/team/group/rls/res/res_pjsip_exten_state.c
https://reviewboard.asterisk.org/r/3723/#comment23462

exten_state_data.device_state_info setup could be simplified this way:
task_data-exten_state_data.device_state_info = 
ao2_bump(info-device_state_info);



/team/group/rls/res/res_pjsip_exten_state.c
https://reviewboard.asterisk.org/r/3723/#comment23464

Off nominal leak of info if exten_state_data-pool creation fails.

Info can be eliminated along with the leak by passing 
exten_state_data-device_state_info instead of info to 
ast_extension_state_extended().

Also it would be better if the assignment inside the if were extracted as 
two statements.
var = func();
if (var  0)



/team/group/rls/res/res_pjsip_mwi.c
https://reviewboard.asterisk.org/r/3723/#comment23465

Extraneous blank line.



/team/group/rls/res/res_pjsip_mwi.c
https://reviewboard.asterisk.org/r/3723/#comment23466

MWI datastore should be a define or global string variable.  As it is, 
the string could have a typo and the compiler cannot catch it.



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23476

AST_VECTOR_APPEND() can fail.  child subscription leaked.



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23472

It might be better to remove_subscription() first before destroying the 
sub_tree contents.

remove_subscription() also does a module unref which would need to be moved 
to unlink the subscription from the list first.



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23467

This is leaked when allocate_subscription() is called below.



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23471

sub_tree leaked?



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23470

sub_tree leaked?



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23469

sub_tree was already added when it was allocated by 
allocate_subscription_tree() above.  This just trashes the linked list.



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23468

sub is leaked here.
sub_tree may be leaked?



/team/group/rls/res/res_pjsip_pubsub.c
https://reviewboard.asterisk.org/r/3723/#comment23477

Use ast_free instead of ast_free_ptr.

You don't need to pass a pointer since the RAII_VAR() macro creates a local 
function that actually calls ast_free().

In fact, using ast_free() allows MALLOC_DEBUG to know where the block was 
actually freed.


- rmudgett


On Aug. 6, 2014, 10:44 a.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3723/
 ---
 
 (Updated Aug. 6, 2014, 10:44 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23869
 https://issues.asterisk.org/jira/browse/ASTERISK-23869
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds NOTIFY support for resource list subscriptions. The way this works 
 is as follows:
 
 When an initial SUBSCRIBE arrives and the subscription tree is built, all 
 leaf nodes are called into in order to generate their initial NOTIFY bodies 
 and store these on their respective subscription nodes.
 
 Sending a NOTIFY requires traversing the tree. List subscriptions will 
 generate a multipart/related body with an RLMI part and parts corresponding 
 to the leaves of the list (at least they will eventually. ASTERISK-23867 
 involves doing this part). Single-resource subscriptions read the stored body 
 on the subscription and uses that to populate the NOTIFY body.
 
 Leaf nodes in the subscription tree, when they have a state change occur, 
 call ast_sip_subscription_notify(), as they previously did. 
 ast_sip_subscription_notify() creates the NOTIFY body for the subscription 
 and stores that on the subscription in the body_text ast_str field. The 
 subscription tree is then told to send a NOTIFY if no batching is enabled or 
 to start a batched NOTIFY if batching is enabled.
 
 When a resource resubscribes or terminates their subscription, a NOTIFY is 
 now automatically generated by the pubsub core instead of calling into 
 subscription handlers. The NOTIFY is built the same way as previously, using 
 stored NOTIFY bodies on the subscription. This NOTIFY can also cause batched 
 notification, when the timer fires, not to actually send their batch since it 
 would be redundant.
 
 You'll notice the code has been

Re: [asterisk-dev] [Code Review] 3889: format: Remove incorrectly assigned format compatibility bits for Opus and VP8.

2014-08-05 Thread rmudgett 


 On Aug. 5, 2014, 8:03 a.m., Joshua Colp wrote:
  Can you elaborate on what actually happens here between 11-12 and 12-12 
  that makes it so we need to remove the newly added ones?

The format compatibility bits need to remain frozen to new codecs because 
chan_iax2 leaks internal implementation values and doesn't know how to 
negotiate any codec options.  Opus has options can be negotiated.  I don't know 
about VP8 but as its a newer codec it likely has negotiable options.

The IAX_IE_CODEC_PREFS body is a sequence of 8 bit values.  In v1.8 and earlier 
the preference values are the index+1 into the frame.c:AST_FORMAT_LIST[].  In 
v10-v11 the preference values are the order that formats are put into the 
format_list ao2 list conatiner in format.c:format_list_init().  The format_list 
is then converted to the format_list_array object and indexed like 
AST_FORMAT_LIST[].  For backwards compatibility, the first 28 match the order 
in v1.8's AST_FORMAT_LIST[] and have format compatibility bits assigned.  There 
is a comment saying the order of the first 28 must not change.  The comment 
does not state why.  However, that order cannot change because those formats 
are in the historical AST_FORMAT_LIST[], those formats have compatibility bits, 
and the index values are sent over the wire by IAX2.  After the fixed formats 
there is a comment saying the order may change.  The comment does not state 
why.  However, the order may change because none of the following formats 
 have a format combatibility bit defined.  In v12 the Opus and VP8 formats were 
added to the format_list eight formats after the end of the fixed formats and 
also assigned format compatibility bits.

Interoperation between v12 and earlier Asterisk version should not be a problem 
because earlier versions doesn't know about Opus and VP8 and thus won't pick 
those codecs.

Between v12 instances, Opus and VP8 could be picked.  However, because of their 
order in the format_list there will be a gap of eight between G719 and Opus 
that will be difficult to maintain without dire comments that won't be 
followed.  Also as stated, IAX2 doesn't know how to negotiate any codec options.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3889/#review12982
---


On Aug. 4, 2014, 5:47 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3889/
 ---
 
 (Updated Aug. 4, 2014, 5:47 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The format compatibility bits need to remain frozen to new codecs.  
 Otherwise, older channel drivers like chan_iax2 could attempt to negotiate 
 them when they have no support for negotiating any of the format's other 
 potential parameters.  In addition, the chan_iax2 format preference order 
 values sent over the wire have no defined fixed value to represent Opus and 
 VP8.
 
 
 Diffs
 -
 
   /branches/12/main/format.c 420025 
 
 Diff: https://reviewboard.asterisk.org/r/3889/diff/
 
 
 Testing
 ---
 
 Compiles and code inspeciton.
 
 See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert().
 The format_list container is used to generate the format_list_array which is 
 then is used by chan_iax2 to determine the format index sent over the wire in 
 IAX_IE_CODEC_PREFS.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3889: format: Remove incorrectly assigned format compatibility bits for Opus and VP8.

2014-08-05 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3889/
---

(Updated Aug. 5, 2014, 12:36 p.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers.


Repository: Asterisk


Description
---

The format compatibility bits need to remain frozen to new codecs.  Otherwise, 
older channel drivers like chan_iax2 could attempt to negotiate them when they 
have no support for negotiating any of the format's other potential parameters. 
 In addition, the chan_iax2 format preference order values sent over the wire 
have no defined fixed value to represent Opus and VP8.


Diffs
-

  /branches/12/main/format.c 420025 

Diff: https://reviewboard.asterisk.org/r/3889/diff/


Testing
---

Compiles and code inspeciton.

See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert().
The format_list container is used to generate the format_list_array which is 
then is used by chan_iax2 to determine the format index sent over the wire in 
IAX_IE_CODEC_PREFS.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3890: chan_iax2: Several media format fixes.

2014-08-05 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3890/
---

(Updated Aug. 5, 2014, 2:39 p.m.)


Review request for Asterisk Developers.


Changes
---

Removed changes from abandoned review https://reviewboard.asterisk.org/r/3889/


Bugs: ASTERISK-24150
https://issues.asterisk.org/jira/browse/ASTERISK-24150


Repository: Asterisk


Description (updated)
---

* Fixed the iax.conf bandwidth option.  This is the root cause of
ASTERISK-24150.

* Added checks in iax2_request() to ensure that there are actual formats
requested for the new channel to prevent any more fracks from issues like
ASTERISK-24150.  This is a consequence of the iax.conf bandwidth option
not working.

* Fixed struct iax2_codec_pref.order member size mismatch issue when
converting to and from the codec preference order list passed over the
wire.  In addition the values sent over the wire are now compatible with
previous Asterisk versions.

* Fixed several issues dealing with the struct iax2_codec_pref members.
Off-by-one, array limit errors, and the order/framing members always need
to be updated together.

* Made iax2_request() setup the channel's native format preference order
according to the user's wishes.  The new media format strategy needs the
order specified earler.

* Fixed usage of ast_format_compatibility_bitfield2format().  The function
can return NULL if the bitfield was not associated with a function.

* Deleted dead code iax2_codec_pref_getsize() and
iax2_codec_pref_setsize().

* Made iax2_parse_allow_disallow() and iax2_codec_pref_string() call
iax2_codec_pref_to_cap() instead of inlining it.

* Made IAX_CAPABILITY_MEDBANDWIDTH, IAX_CAPABILITY_LOWBANDWIDTH, and
IAX_CAPABILITY_LOWFREE constants again as they were in Asterisk v1.8.

* Renamed prefs to prefs_global so it won't get confused with the local
pref versions.

* Fixed too small buffer in handle_cli_iax2_show_peer().

* Fixed ast_cli() calls in handle_cli_iax2_show_peer() to output complete
lines.

* Changed struct create_addr_info.prefs to be struct iax2_codec_pref as an
optimization so iax2_request() and iax2_call() do less work.

* Fixed a potential deadlock in ast_iax2_new() on an off-nominal path when
the pbx could not get started.

* Made set_config() setup a local prefs list along side the local
capability format bitfield.  Once the config is loaded, then the local
copies are put into the global versions.

* Fix unininialized codec_buf in function_iaxpeer().


Diffs (updated)
-

  /trunk/main/format_compatibility.c 420087 
  /trunk/include/asterisk/format_compatibility.h 420087 
  /trunk/channels/iax2/include/format_compatibility.h 420087 
  /trunk/channels/iax2/include/codec_pref.h 420087 
  /trunk/channels/iax2/format_compatibility.c 420087 
  /trunk/channels/iax2/codec_pref.c 420087 
  /trunk/channels/chan_iax2.c 420087 

Diff: https://reviewboard.asterisk.org/r/3890/diff/


Testing
---

* The iax.conf bandwidth option is now functional and no longer causes a frack 
when a call is made.

* The CLI iax2 show peer has enough buffer space to generate a longer codec 
list for the configured codecs header.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3890: chan_iax2: Several media format fixes.

2014-08-05 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3890/
---

(Updated Aug. 5, 2014, 5:51 p.m.)


Review request for Asterisk Developers.


Bugs: ASTERISK-24150
https://issues.asterisk.org/jira/browse/ASTERISK-24150


Repository: Asterisk


Description
---

* Fixed the iax.conf bandwidth option.  This is the root cause of
ASTERISK-24150.

* Added checks in iax2_request() to ensure that there are actual formats
requested for the new channel to prevent any more fracks from issues like
ASTERISK-24150.  This is a consequence of the iax.conf bandwidth option
not working.

* Fixed struct iax2_codec_pref.order member size mismatch issue when
converting to and from the codec preference order list passed over the
wire.  In addition the values sent over the wire are now compatible with
previous Asterisk versions.

* Fixed several issues dealing with the struct iax2_codec_pref members.
Off-by-one, array limit errors, and the order/framing members always need
to be updated together.

* Made iax2_request() setup the channel's native format preference order
according to the user's wishes.  The new media format strategy needs the
order specified earler.

* Fixed usage of ast_format_compatibility_bitfield2format().  The function
can return NULL if the bitfield was not associated with a function.

* Deleted dead code iax2_codec_pref_getsize() and
iax2_codec_pref_setsize().

* Made iax2_parse_allow_disallow() and iax2_codec_pref_string() call
iax2_codec_pref_to_cap() instead of inlining it.

* Made IAX_CAPABILITY_MEDBANDWIDTH, IAX_CAPABILITY_LOWBANDWIDTH, and
IAX_CAPABILITY_LOWFREE constants again as they were in Asterisk v1.8.

* Renamed prefs to prefs_global so it won't get confused with the local
pref versions.

* Fixed too small buffer in handle_cli_iax2_show_peer().

* Fixed ast_cli() calls in handle_cli_iax2_show_peer() to output complete
lines.

* Changed struct create_addr_info.prefs to be struct iax2_codec_pref as an
optimization so iax2_request() and iax2_call() do less work.

* Fixed a potential deadlock in ast_iax2_new() on an off-nominal path when
the pbx could not get started.

* Made set_config() setup a local prefs list along side the local
capability format bitfield.  Once the config is loaded, then the local
copies are put into the global versions.

* Fix unininialized codec_buf in function_iaxpeer().


Diffs
-

  /trunk/main/format_compatibility.c 420087 
  /trunk/include/asterisk/format_compatibility.h 420087 
  /trunk/channels/iax2/include/format_compatibility.h 420087 
  /trunk/channels/iax2/include/codec_pref.h 420087 
  /trunk/channels/iax2/format_compatibility.c 420087 
  /trunk/channels/iax2/codec_pref.c 420087 
  /trunk/channels/chan_iax2.c 420087 

Diff: https://reviewboard.asterisk.org/r/3890/diff/


Testing (updated)
---

* The 12 testsuite tests tagged with iax2 pass.

* The iax.conf bandwidth option is now functional and no longer causes a frack 
when a call is made.

* The CLI iax2 show peer has enough buffer space to generate a longer codec 
list for the configured codecs header.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3882: Replace sip_tls_read() and resolve the large SDP poll issue

2014-08-04 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3882/#review12971
---



trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3882/#comment23363

Should also test for EINTR and do the same thing as for EAGAIN.


- rmudgett


On July 31, 2014, 1:14 p.m., ebroad wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3882/
 ---
 
 (Updated July 31, 2014, 1:14 p.m.)
 
 
 Review request for Asterisk Developers and Alexander Traud.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Replace sip_tls_read() and sip_tcp_read() with a single function and resolve 
 the poll/wait issue with large SDP payloads. See 
 https://reviewboard.asterisk.org/r/3653/ for the discussion on this.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 419821 
 
 Diff: https://reviewboard.asterisk.org/r/3882/diff/
 
 
 Testing
 ---
 
 Made and received calls successfully with CSipSimple with full SIP headers 
 over TLS, SRTP and multiple codecs enabled ensuring a large SDP payload.
 
 
 Thanks,
 
 ebroad
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3870: alembic: Adjust sippeers, queue_members, and voicemail_messages tables.

2014-08-04 Thread rmudgett 


 On Aug. 4, 2014, 4:10 p.m., Mark Michelson wrote:
  /branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py,
   lines 36-37
  https://reviewboard.asterisk.org/r/3870/diff/1/?file=65734#file65734line36
 
  There is something just absolutely frightening about this. Can you add 
  to your comment:
  
  1) why this is necessary
  2) a reference indicating that this is the correct way of doing this 
  for MySQL
  3) some reassurance that this will not do something awful for other 
  DBMSs.
  
  As it looks, every voicemail message is going to have 4GB set aside for 
  it.

This is necessary because mysql will truncate recordings to about 4 seconds 
worth of audio as mysql limits a BLOB to only 64k.
For postgres, the backend did not output anything different.

This is the only parameter that LargeBinary is documented as accepting.
See LargeBinary http://docs.sqlalchemy.org/en/rel_0_9/core/types.html

I'll augment the comment.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3870/#review12974
---


On July 30, 2014, 9:52 a.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3870/
 ---
 
 (Updated July 30, 2014, 9:52 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23825, ASTERISK-23847 and ASTERISK-23909
 https://issues.asterisk.org/jira/browse/ASTERISK-23825
 https://issues.asterisk.org/jira/browse/ASTERISK-23847
 https://issues.asterisk.org/jira/browse/ASTERISK-23909
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 * Increased the sippeers useragent max string size to 255.
 
 * Changed the queue_members uniqueid to an auto incremented integer instead 
 of a string.
 
 * Increased the voicemail_messages BLOB size to LONGBLOB on mysql.
 
 * Fixed the add_tables_for_pjsip config change version downgrade actions to 
 drop a table it created.
 
 * Adjusted the sample alembic.ini files cdr.ini.sample, config.ini.sample, 
 and voicemail.ini.sample to give a mysql and postgres sqlalchemy.url lines.
 
 
 Diffs
 -
 
   
 /branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py
  PRE-CREATION 
   /branches/12/contrib/ast-db-manage/voicemail.ini.sample 419804 
   
 /branches/12/contrib/ast-db-manage/config/versions/5139253c0423_make_q_member_uniqueid_autoinc.py
  PRE-CREATION 
   
 /branches/12/contrib/ast-db-manage/config/versions/43956d550a44_add_tables_for_pjsip.py
  419804 
   
 /branches/12/contrib/ast-db-manage/config/versions/1758e8bbf6b_increase_useragent_column_size.py
  PRE-CREATION 
   /branches/12/contrib/ast-db-manage/config.ini.sample 419804 
   /branches/12/contrib/ast-db-manage/cdr.ini.sample 419804 
 
 Diff: https://reviewboard.asterisk.org/r/3870/diff/
 
 
 Testing
 ---
 
 The alembic table adjustments work in the upgrade and downgrade modes for 
 mysql and postgres.
 
 The queue_members change does give a postgres warning message from the 
 postgres alembic backend about a mysql only kind of change.  The warning is 
 benign and can be ignored as it is an alembic only warning.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3870: alembic: Adjust sippeers, queue_members, and voicemail_messages tables.

2014-08-04 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3870/
---

(Updated Aug. 4, 2014, 4:39 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed feedback.


Bugs: ASTERISK-23825, ASTERISK-23847 and ASTERISK-23909
https://issues.asterisk.org/jira/browse/ASTERISK-23825
https://issues.asterisk.org/jira/browse/ASTERISK-23847
https://issues.asterisk.org/jira/browse/ASTERISK-23909


Repository: Asterisk


Description
---

* Increased the sippeers useragent max string size to 255.

* Changed the queue_members uniqueid to an auto incremented integer instead of 
a string.

* Increased the voicemail_messages BLOB size to LONGBLOB on mysql.

* Fixed the add_tables_for_pjsip config change version downgrade actions to 
drop a table it created.

* Adjusted the sample alembic.ini files cdr.ini.sample, config.ini.sample, and 
voicemail.ini.sample to give a mysql and postgres sqlalchemy.url lines.


Diffs (updated)
-

  
/branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py
 PRE-CREATION 
  /branches/12/contrib/ast-db-manage/voicemail.ini.sample 420024 
  
/branches/12/contrib/ast-db-manage/config/versions/5139253c0423_make_q_member_uniqueid_autoinc.py
 PRE-CREATION 
  
/branches/12/contrib/ast-db-manage/config/versions/43956d550a44_add_tables_for_pjsip.py
 420024 
  
/branches/12/contrib/ast-db-manage/config/versions/1758e8bbf6b_increase_useragent_column_size.py
 PRE-CREATION 
  /branches/12/contrib/ast-db-manage/config.ini.sample 420024 
  /branches/12/contrib/ast-db-manage/cdr.ini.sample 420024 

Diff: https://reviewboard.asterisk.org/r/3870/diff/


Testing
---

The alembic table adjustments work in the upgrade and downgrade modes for mysql 
and postgres.

The queue_members change does give a postgres warning message from the postgres 
alembic backend about a mysql only kind of change.  The warning is benign and 
can be ignored as it is an alembic only warning.


Thanks,

rmudgett

-- 
_
-- 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

[asterisk-dev] [Code Review] 3889: format: Remove incorrectly assigned format compatibility bits for Opus and VP8.

2014-08-04 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3889/
---

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

The format compatibility bits need to remain frozen to new codecs.  Otherwise, 
older channel drivers like chan_iax2 could attempt to negotiate them when they 
have no support for negotiating any of the format's other potential parameters. 
 In addition, the chan_iax2 format preference order values sent over the wire 
have no defined fixed value to represent Opus and VP8.


Diffs
-

  /branches/12/main/format.c 420025 

Diff: https://reviewboard.asterisk.org/r/3889/diff/


Testing
---

Compiles and code inspeciton.

See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert().
The format_list container is used to generate the format_list_array which is 
then is used by chan_iax2 to determine the format index sent over the wire in 
IAX_IE_CODEC_PREFS.


Thanks,

rmudgett

-- 
_
-- 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

[asterisk-dev] [Code Review] 3890: chan_iax2: Several media format fixes.

2014-08-04 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3890/
---

Review request for Asterisk Developers.


Bugs: ASTERISK-24150
https://issues.asterisk.org/jira/browse/ASTERISK-24150


Repository: Asterisk


Description
---

* Fixed the iax.conf bandwidth option.  This is the root cause of
ASTERISK-24150.

* Added checks in iax2_request() to ensure that there are actual formats
requested for the new channel to prevent any more fracks from issues like
ASTERISK-24150.  This is a consequence of the iax.conf bandwidth option
not working.

* Fixed struct iax2_codec_pref.order member size mismatch issue when
converting to and from the codec preference order list passed over the
wire.  In addition the values sent over the wire are now compatible with
previous Asterisk versions.

* Fixed several issues dealing with the struct iax2_codec_pref members.
Off-by-one, array limit errors, and the order/framing members always need
to be updated together.

* Made iax2_request() setup the channel's native format preference order
according to the user's wishes.  The new media format strategy needs the
order specified earler.

* Fixed usage of ast_format_compatibility_bitfield2format().  The function
can return NULL if the bitfield was not associated with a function.

* Deleted dead code iax2_codec_pref_getsize() and
iax2_codec_pref_setsize().

* Made iax2_parse_allow_disallow() and iax2_codec_pref_string() call
iax2_codec_pref_to_cap() instead of inlining it.

* Made IAX_CAPABILITY_MEDBANDWIDTH, IAX_CAPABILITY_LOWBANDWIDTH, and
IAX_CAPABILITY_LOWFREE constants again as they were in Asterisk v1.8.

* Renamed prefs to prefs_global so it won't get confused with the local
pref versions.

* Fixed too small buffer in handle_cli_iax2_show_peer().

* Fixed ast_cli() calls in handle_cli_iax2_show_peer() to output complete
lines.

* Changed struct create_addr_info.prefs to be struct iax2_codec_pref as an
optimization so iax2_request() and iax2_call() do less work.

* Fixed a potential deadlock in ast_iax2_new() on an off-nominal path when
the pbx could not get started.

* Made set_config() setup a local prefs list along side the local
capability format bitfield.  Once the config is loaded, then the local
copies are put into the global versions.

* Fix unininialized codec_buf in function_iaxpeer().

This review includes the changes in
https://reviewboard.asterisk.org/r/3889/ when merged to trunk and the
conflicts fixed for this patch.


Diffs
-

  /trunk/main/format_compatibility.c 420026 
  /trunk/include/asterisk/format_compatibility.h 420026 
  /trunk/channels/iax2/include/format_compatibility.h 420026 
  /trunk/channels/iax2/include/codec_pref.h 420026 
  /trunk/channels/iax2/format_compatibility.c 420026 
  /trunk/channels/iax2/codec_pref.c 420026 
  /trunk/channels/chan_iax2.c 420026 

Diff: https://reviewboard.asterisk.org/r/3890/diff/


Testing
---

* The iax.conf bandwidth option is now functional and no longer causes a frack 
when a call is made.

* The CLI iax2 show peer has enough buffer space to generate a longer codec 
list for the configured codecs header.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3890: chan_iax2: Several media format fixes.

2014-08-04 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3890/#review12976
---



/trunk/channels/iax2/include/codec_pref.h
https://reviewboard.asterisk.org/r/3890/#comment23380

Fixed comment to read:
Array is ordered by preference.  Contains the iax2_supported_formats[] 
index + 1.


- rmudgett


On Aug. 4, 2014, 7:41 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3890/
 ---
 
 (Updated Aug. 4, 2014, 7:41 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24150
 https://issues.asterisk.org/jira/browse/ASTERISK-24150
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 * Fixed the iax.conf bandwidth option.  This is the root cause of
 ASTERISK-24150.
 
 * Added checks in iax2_request() to ensure that there are actual formats
 requested for the new channel to prevent any more fracks from issues like
 ASTERISK-24150.  This is a consequence of the iax.conf bandwidth option
 not working.
 
 * Fixed struct iax2_codec_pref.order member size mismatch issue when
 converting to and from the codec preference order list passed over the
 wire.  In addition the values sent over the wire are now compatible with
 previous Asterisk versions.
 
 * Fixed several issues dealing with the struct iax2_codec_pref members.
 Off-by-one, array limit errors, and the order/framing members always need
 to be updated together.
 
 * Made iax2_request() setup the channel's native format preference order
 according to the user's wishes.  The new media format strategy needs the
 order specified earler.
 
 * Fixed usage of ast_format_compatibility_bitfield2format().  The function
 can return NULL if the bitfield was not associated with a function.
 
 * Deleted dead code iax2_codec_pref_getsize() and
 iax2_codec_pref_setsize().
 
 * Made iax2_parse_allow_disallow() and iax2_codec_pref_string() call
 iax2_codec_pref_to_cap() instead of inlining it.
 
 * Made IAX_CAPABILITY_MEDBANDWIDTH, IAX_CAPABILITY_LOWBANDWIDTH, and
 IAX_CAPABILITY_LOWFREE constants again as they were in Asterisk v1.8.
 
 * Renamed prefs to prefs_global so it won't get confused with the local
 pref versions.
 
 * Fixed too small buffer in handle_cli_iax2_show_peer().
 
 * Fixed ast_cli() calls in handle_cli_iax2_show_peer() to output complete
 lines.
 
 * Changed struct create_addr_info.prefs to be struct iax2_codec_pref as an
 optimization so iax2_request() and iax2_call() do less work.
 
 * Fixed a potential deadlock in ast_iax2_new() on an off-nominal path when
 the pbx could not get started.
 
 * Made set_config() setup a local prefs list along side the local
 capability format bitfield.  Once the config is loaded, then the local
 copies are put into the global versions.
 
 * Fix unininialized codec_buf in function_iaxpeer().
 
 This review includes the changes in
 https://reviewboard.asterisk.org/r/3889/ when merged to trunk and the
 conflicts fixed for this patch.
 
 
 Diffs
 -
 
   /trunk/main/format_compatibility.c 420026 
   /trunk/include/asterisk/format_compatibility.h 420026 
   /trunk/channels/iax2/include/format_compatibility.h 420026 
   /trunk/channels/iax2/include/codec_pref.h 420026 
   /trunk/channels/iax2/format_compatibility.c 420026 
   /trunk/channels/iax2/codec_pref.c 420026 
   /trunk/channels/chan_iax2.c 420026 
 
 Diff: https://reviewboard.asterisk.org/r/3890/diff/
 
 
 Testing
 ---
 
 * The iax.conf bandwidth option is now functional and no longer causes a 
 frack when a call is made.
 
 * The CLI iax2 show peer has enough buffer space to generate a longer codec 
 list for the configured codecs header.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3653: chan_sip: (Optionally) poll even on first part of TLS message

2014-07-31 Thread rmudgett 
 specifically targets TCP connections and not TLS. TLS 
 connections
  were not reported as having the issue, plus changing TLS would be a 
 much more
  invasive operation.
  
  In my opinion, we should remove the use of FILE handles altogether in 
 the TCP/TLS
  code [...]
 
 Yes. There still is fgets() in chan_sip.c, it should be killed.
 
 rmudgett wrote:
 With the recent security fix that affected TCP/TLS with HTTP, eliminating 
 the fgets() usage is much easier.  The SSL_read/SSL_write functions are used 
 correctly (or used more correctly :) ) with that change.  It should be fairly 
 straight forward now to change the code to use the sip_tcp_read instead and 
 remove sip_tls_read with its weird need_poll/after_poll flags.  The 
 sip_tcp_read uses the mentioned tcptls_session-overflow_buf since that 
 overflow buffer was created specifically for the sip_tcp_read function.  Even 
 better would be to change chan_sip to not use the tcptls_session-f pointer 
 at all and call the ast_tcptls_server_read() and ast_tcptls_server_write() 
 functions instead.
 
 ebroad wrote:
 I added a patch to the issue in Jira which replaces sip_tls_read() and 
 sip_tcp_read() with a single sip_tcptls_read(), which is a copy of 
 sip_tcp_read(), except that it utilizes ast_tcptls_server_read(). My only 
 concern is the while loop which handles EAGAIN, it could block infinitely, 
 any suggestions on how to handle this? I am considering adding a counter that 
 breaks the loop once it is equal to timeout..

In this case the EAGAIN can be interpreted as we didn't get any data.  Return 
and let the caller poll/wait for more data to come in.  When you're ready put 
that patch on this review as the next revision.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3653/#review12371
---


On June 20, 2014, 9:06 a.m., Alexander Traud wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3653/
 ---
 
 (Updated June 20, 2014, 9:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With some large SDP, a *second* poll is required on the first part of a TLS 
 message.
 
 The current code did not poll a second time because the variable need_poll 
 was inited with yes (1). That poll was a no-operation because there was a 
 socket event already (which mandates fgets without poll). In the current 
 code, poll returned immediately, fgets returned NULL, after_poll was yes (1), 
 sip_tls_read returned failed (-1), _sip_tcp_helper_thread went to cleanup, 
 called ast_tcptls_close_session_file, which closed the TLS connection.
 
 The proposed patch, reads the gets the first message. If that failed, it does 
 poll. This fixed all large SDP issues with SIP over TLS which I faced.
 
 I am aware there were changes committed to tcptls.c just recently (revision 
 415907). Anyway, let us fix this bug as well.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 416319 
 
 Diff: https://reviewboard.asterisk.org/r/3653/diff/
 
 
 Testing
 ---
 
 Asterisk 12.3
 
 
 Thanks,
 
 Alexander Traud
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3653: chan_sip: (Optionally) poll even on first part of TLS message

2014-07-31 Thread rmudgett 
 specifically targets TCP connections and not TLS. TLS 
 connections
  were not reported as having the issue, plus changing TLS would be a 
 much more
  invasive operation.
  
  In my opinion, we should remove the use of FILE handles altogether in 
 the TCP/TLS
  code [...]
 
 Yes. There still is fgets() in chan_sip.c, it should be killed.
 
 rmudgett wrote:
 With the recent security fix that affected TCP/TLS with HTTP, eliminating 
 the fgets() usage is much easier.  The SSL_read/SSL_write functions are used 
 correctly (or used more correctly :) ) with that change.  It should be fairly 
 straight forward now to change the code to use the sip_tcp_read instead and 
 remove sip_tls_read with its weird need_poll/after_poll flags.  The 
 sip_tcp_read uses the mentioned tcptls_session-overflow_buf since that 
 overflow buffer was created specifically for the sip_tcp_read function.  Even 
 better would be to change chan_sip to not use the tcptls_session-f pointer 
 at all and call the ast_tcptls_server_read() and ast_tcptls_server_write() 
 functions instead.
 
 ebroad wrote:
 I added a patch to the issue in Jira which replaces sip_tls_read() and 
 sip_tcp_read() with a single sip_tcptls_read(), which is a copy of 
 sip_tcp_read(), except that it utilizes ast_tcptls_server_read(). My only 
 concern is the while loop which handles EAGAIN, it could block infinitely, 
 any suggestions on how to handle this? I am considering adding a counter that 
 breaks the loop once it is equal to timeout..
 
 rmudgett wrote:
 In this case the EAGAIN can be interpreted as we didn't get any data.  
 Return and let the caller poll/wait for more data to come in.  When you're 
 ready put that patch on this review as the next revision.
 
 ebroad wrote:
 Thanks! To clarify, get rid of the loop in sip_tcptls_read() and move 
 it(or something like it) to *_sip_tcp_helper_thread()?

After looking at the sip_tcp_read() function when you get an EAGAIN just do a 
continue that goes to the top of the main while loop in sip_tcp_read().  The 
loop is already there with a timeout since you are replacing the recv() with 
the ast_tcptls_server_read() call.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3653/#review12371
---


On June 20, 2014, 9:06 a.m., Alexander Traud wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3653/
 ---
 
 (Updated June 20, 2014, 9:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With some large SDP, a *second* poll is required on the first part of a TLS 
 message.
 
 The current code did not poll a second time because the variable need_poll 
 was inited with yes (1). That poll was a no-operation because there was a 
 socket event already (which mandates fgets without poll). In the current 
 code, poll returned immediately, fgets returned NULL, after_poll was yes (1), 
 sip_tls_read returned failed (-1), _sip_tcp_helper_thread went to cleanup, 
 called ast_tcptls_close_session_file, which closed the TLS connection.
 
 The proposed patch, reads the gets the first message. If that failed, it does 
 poll. This fixed all large SDP issues with SIP over TLS which I faced.
 
 I am aware there were changes committed to tcptls.c just recently (revision 
 415907). Anyway, let us fix this bug as well.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 416319 
 
 Diff: https://reviewboard.asterisk.org/r/3653/diff/
 
 
 Testing
 ---
 
 Asterisk 12.3
 
 
 Thanks,
 
 Alexander Traud
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3653: chan_sip: (Optionally) poll even on first part of TLS message

2014-07-31 Thread rmudgett 
 specifically targets TCP connections and not TLS. TLS 
 connections
  were not reported as having the issue, plus changing TLS would be a 
 much more
  invasive operation.
  
  In my opinion, we should remove the use of FILE handles altogether in 
 the TCP/TLS
  code [...]
 
 Yes. There still is fgets() in chan_sip.c, it should be killed.
 
 rmudgett wrote:
 With the recent security fix that affected TCP/TLS with HTTP, eliminating 
 the fgets() usage is much easier.  The SSL_read/SSL_write functions are used 
 correctly (or used more correctly :) ) with that change.  It should be fairly 
 straight forward now to change the code to use the sip_tcp_read instead and 
 remove sip_tls_read with its weird need_poll/after_poll flags.  The 
 sip_tcp_read uses the mentioned tcptls_session-overflow_buf since that 
 overflow buffer was created specifically for the sip_tcp_read function.  Even 
 better would be to change chan_sip to not use the tcptls_session-f pointer 
 at all and call the ast_tcptls_server_read() and ast_tcptls_server_write() 
 functions instead.
 
 ebroad wrote:
 I added a patch to the issue in Jira which replaces sip_tls_read() and 
 sip_tcp_read() with a single sip_tcptls_read(), which is a copy of 
 sip_tcp_read(), except that it utilizes ast_tcptls_server_read(). My only 
 concern is the while loop which handles EAGAIN, it could block infinitely, 
 any suggestions on how to handle this? I am considering adding a counter that 
 breaks the loop once it is equal to timeout..
 
 rmudgett wrote:
 In this case the EAGAIN can be interpreted as we didn't get any data.  
 Return and let the caller poll/wait for more data to come in.  When you're 
 ready put that patch on this review as the next revision.
 
 ebroad wrote:
 Thanks! To clarify, get rid of the loop in sip_tcptls_read() and move 
 it(or something like it) to *_sip_tcp_helper_thread()?
 
 rmudgett wrote:
 After looking at the sip_tcp_read() function when you get an EAGAIN just 
 do a continue that goes to the top of the main while loop in sip_tcp_read().  
 The loop is already there with a timeout since you are replacing the recv() 
 with the ast_tcptls_server_read() call.
 
 ebroad wrote:
 I think the issue needs to be reopened for me to post a revision. Thanks!

Oh.  I thought you were the original poster of the review.  You'll need to 
create a new review then and put the link in a comment of the issue.  This 
issue would then need to be discarded in favor of the new review.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3653/#review12371
---


On June 20, 2014, 9:06 a.m., Alexander Traud wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3653/
 ---
 
 (Updated June 20, 2014, 9:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-18345
 https://issues.asterisk.org/jira/browse/ASTERISK-18345
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With some large SDP, a *second* poll is required on the first part of a TLS 
 message.
 
 The current code did not poll a second time because the variable need_poll 
 was inited with yes (1). That poll was a no-operation because there was a 
 socket event already (which mandates fgets without poll). In the current 
 code, poll returned immediately, fgets returned NULL, after_poll was yes (1), 
 sip_tls_read returned failed (-1), _sip_tcp_helper_thread went to cleanup, 
 called ast_tcptls_close_session_file, which closed the TLS connection.
 
 The proposed patch, reads the gets the first message. If that failed, it does 
 poll. This fixed all large SDP issues with SIP over TLS which I faced.
 
 I am aware there were changes committed to tcptls.c just recently (revision 
 415907). Anyway, let us fix this bug as well.
 
 
 Diffs
 -
 
   trunk/channels/chan_sip.c 416319 
 
 Diff: https://reviewboard.asterisk.org/r/3653/diff/
 
 
 Testing
 ---
 
 Asterisk 12.3
 
 
 Thanks,
 
 Alexander Traud
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3603: func_jitterbuffer: fix errors and leaks caused by certain masquerade's

2014-07-30 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3603/#review12921
---



/branches/11/funcs/func_jitterbuffer.c
https://reviewboard.asterisk.org/r/3603/#comment23297

And function curly on its own line.


- rmudgett


On June 13, 2014, 1:48 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3603/
 ---
 
 (Updated June 13, 2014, 1:48 p.m.)
 
 
 Review request for Asterisk Developers, Joshua Colp and Matt Jordan.
 
 
 Bugs: ASTERISK-22409
 https://issues.asterisk.org/jira/browse/ASTERISK-22409
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 During masquerade it is possible for the AST_JITTERBUFFER_FD to be cleared 
 (set to -1).  This change adds a check when copying channel fd's to prevent 
 clearing an FD with -1.  This seems to resolve the bad audio quality 
 experienced after the masquerade.  When AST_JITTERBUFFER_FD was set to -1, 
 this prevented the channel from polling that timer.  This caused RTP packets 
 to be received late, and discarded.
 
 The changes to func_jitterbuffer.c were created first.  ast_free(jbframe); is 
 needed to prevent jbframe from leaking if it is rejected by jb_impl.  This 
 ensures we don't start leaking packets if they are received too late or 
 rejected by jb_impl for any other reason.
 
 The second change to func_jitterbuffer prevents a leak of ast_null_frame's 
 that were duplicated (ie with ast_frdup or ast_frisolate).  I believe this 
 leak might actually be unrelated to the masquerade issue, and likely occurs 
 for every single ast_null_frame.
 
 
 Diffs
 -
 
   /branches/11/main/channel.c 416102 
   /branches/11/funcs/func_jitterbuffer.c 416102 
 
 Diff: https://reviewboard.asterisk.org/r/3603/diff/
 
 
 Testing
 ---
 
 Verified the scenario outlined in ASTERISK-22409 no longer experiences audio 
 quality loss, and no longer causes leaks (tested under valgrind).  I patched 
 asterisk to ensure that ast_frfree performed an immediate free to ensure 
 valgrind would report any attempted use after free.
 
 In early testing, I used debug messages instead of the added ast_frfree's - I 
 verified the leaked frames reported by valgrind matched exactly to the number 
 of debug messages.
 
 For the masquerade fix I tested with some debug code that showed the old and 
 new FD, this is how I found the valid FD being replaced by -1.  See JIRA 
 ticket for example output.
 
 I have not tested this issue or fix against 12+, but the relevant code is the 
 same as 11 - func_jitterbuffer code was moved to core but still the same code.
 
 
 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

Re: [asterisk-dev] [Code Review] 3833: Allow sending voicemail to multiple email addresses

2014-07-30 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3833/#review12923
---



trunk/apps/app_voicemail.c
https://reviewboard.asterisk.org/r/3833/#comment23301

free() and ast_free() are NULL tolerant.
However, it is likely better to set vmu-email = NULL after freeing like 
other places.


- rmudgett


On July 21, 2014, 4:54 p.m., Jason Parker wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3833/
 ---
 
 (Updated July 21, 2014, 4:54 p.m.)
 
 
 Review request for Asterisk Developers and Jacob Barber.
 
 
 Bugs: ASTERISK-24045
 https://issues.asterisk.org/jira/browse/ASTERISK-24045
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Allow sending voicemail to multiple email addresses.
 
 Should this supersede https://reviewboard.asterisk.org/r/3829/ ?  I would 
 think so.
 
 
 Diffs
 -
 
   trunk/apps/app_voicemail.c 404951 
 
 Diff: https://reviewboard.asterisk.org/r/3833/diff/
 
 
 Testing
 ---
 
 Patch in FreePBX distro for ~6 months.
 
 
 Thanks,
 
 Jason Parker
 


-- 
_
-- 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

[asterisk-dev] [Code Review] 3870: alembic: Adjust sippeers, queue_members, and voicemail_messages tables.

2014-07-30 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3870/
---

Review request for Asterisk Developers.


Bugs: ASTERISK-23825, ASTERISK-23847 and ASTERISK-23909
https://issues.asterisk.org/jira/browse/ASTERISK-23825
https://issues.asterisk.org/jira/browse/ASTERISK-23847
https://issues.asterisk.org/jira/browse/ASTERISK-23909


Repository: Asterisk


Description
---

* Increased the sippeers useragent max string size to 255.

* Changed the queue_members uniqueid to an auto incremented integer instead of 
a string.

* Increased the voicemail_messages BLOB size to LONGBLOB on mysql.

* Fixed the add_tables_for_pjsip config change version downgrade actions to 
drop a table it created.

* Adjusted the sample alembic.ini files cdr.ini.sample, config.ini.sample, and 
voicemail.ini.sample to give a mysql and postgres sqlalchemy.url lines.


Diffs
-

  
/branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py
 PRE-CREATION 
  /branches/12/contrib/ast-db-manage/voicemail.ini.sample 419804 
  
/branches/12/contrib/ast-db-manage/config/versions/5139253c0423_make_q_member_uniqueid_autoinc.py
 PRE-CREATION 
  
/branches/12/contrib/ast-db-manage/config/versions/43956d550a44_add_tables_for_pjsip.py
 419804 
  
/branches/12/contrib/ast-db-manage/config/versions/1758e8bbf6b_increase_useragent_column_size.py
 PRE-CREATION 
  /branches/12/contrib/ast-db-manage/config.ini.sample 419804 
  /branches/12/contrib/ast-db-manage/cdr.ini.sample 419804 

Diff: https://reviewboard.asterisk.org/r/3870/diff/


Testing
---

The alembic table adjustments work in the upgrade and downgrade modes for mysql 
and postgres.

The queue_members change does give a postgres warning message from the postgres 
alembic backend about a mysql only kind of change.  The warning is benign and 
can be ignored as it is an alembic only warning.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3865: Stasis: Handle incoming masquerades

2014-07-28 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3865/#review12884
---



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3865/#comment23251

The ao2_unlink likely will have a problem finding the object if the key has 
changed before it was removed.  You likely will have to do an ao2_callback of 
the container to actually find and remove it.

Could use the ao2_callback that finds the control to unlink it as well.

During the masquerade, the app_controls container is broken while the 
object key indicates it should be in a different location of the container.  An 
ao2_container integrity check will fail at this time if the checks are enabled.



team/group/ari-greedy-atxfer/res/stasis/app.c
https://reviewboard.asterisk.org/r/3865/#comment23250

Is OBJ_NOLOCK needed here?  It wasn't used on the ao2_find above.


- rmudgett


On July 28, 2014, 9:56 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3865/
 ---
 
 (Updated July 28, 2014, 9:56 a.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds handling for a channel being pushed into Stasis() via masquerade. 
 It notifies the Stasis() application using the previously established 
 StasisStart with the replace_channel key populated with a channel snapshot 
 of the channel that is being replaced. It also sets up the new channel topic 
 forwards required to get information from the new channel and tears down the 
 old channel topic forwards.
 
 This patch also introduces the concept of chan_breakdown datastore callbacks 
 which are called for the channel being masqueraded into during a masquerade.
 
 
 Diffs
 -
 
   team/group/ari-greedy-atxfer/res/stasis/app.c 419681 
   team/group/ari-greedy-atxfer/res/stasis/app.h 419681 
   team/group/ari-greedy-atxfer/res/res_stasis.c 419681 
   team/group/ari-greedy-atxfer/main/channel.c 419681 
   team/group/ari-greedy-atxfer/include/asterisk/datastore.h 419681 
 
 Diff: https://reviewboard.asterisk.org/r/3865/diff/
 
 
 Testing
 ---
 
 Verified that the correct messages were being received by the Stasis() 
 application monitoring the channel.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3865: Stasis: Handle incoming masquerades

2014-07-28 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3865/#review12886
---



team/group/ari-greedy-atxfer/res/stasis/app.c
https://reviewboard.asterisk.org/r/3865/#comment23255

Use of OBJ_NOLOCK is required _only_ if the container has a rd/wr lick.  
Otherwise it is an optimization at best.  You are telling the ao2 function that 
you have alread obtained a lock on the container.

The app-forwards container has a recursive mutex so OBJ_NOLOCK really 
should not be used at all.

In the case of this patch have you obtained it?  I don't think so.


- rmudgett


On July 28, 2014, 11:41 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3865/
 ---
 
 (Updated July 28, 2014, 11:41 a.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds handling for a channel being pushed into Stasis() via masquerade. 
 It notifies the Stasis() application using the previously established 
 StasisStart with the replace_channel key populated with a channel snapshot 
 of the channel that is being replaced. It also sets up the new channel topic 
 forwards required to get information from the new channel and tears down the 
 old channel topic forwards.
 
 This patch also introduces the concept of chan_breakdown datastore callbacks 
 which are called for the channel being masqueraded into during a masquerade.
 
 
 Diffs
 -
 
   team/group/ari-greedy-atxfer/res/stasis/app.c 419681 
   team/group/ari-greedy-atxfer/res/stasis/app.h 419681 
   team/group/ari-greedy-atxfer/res/res_stasis.c 419681 
   team/group/ari-greedy-atxfer/main/channel.c 419681 
   team/group/ari-greedy-atxfer/include/asterisk/datastore.h 419681 
 
 Diff: https://reviewboard.asterisk.org/r/3865/diff/
 
 
 Testing
 ---
 
 Verified that the correct messages were being received by the Stasis() 
 application monitoring the channel.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3859: datastores: Audit v1.8 ast_channel_datastore_remove usage.

2014-07-28 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3859/
---

(Updated July 28, 2014, 1:27 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419684


Repository: Asterisk


Description
---

Audit of v1.8 usage of ast_channel_datastore_remove() for datastore memory 
leaks.

* Fixed leaks in app_speech_utils and func_frame_trace.

* Fixed app_speech_utils not locking the channel when accessing the channel 
datastore list.


Diffs
-

  /branches/1.8/funcs/func_frame_trace.c 419627 
  /branches/1.8/apps/app_speech_utils.c 419627 
  /branches/1.8/apps/app_queue.c 419627 

Diff: https://reviewboard.asterisk.org/r/3859/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3860: datastores: Audit v11 ast_channel_datastore_remove usage.

2014-07-28 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3860/
---

(Updated July 28, 2014, 6:34 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419685


Repository: Asterisk


Description
---

Audit of v11 usage of ast_channel_datastore_remove() for datastore memory leaks.

* Fixed leak in func_jitterbuffer.

This is the additions for v11 over the v1.8 audit at 
https://reviewboard.asterisk.org/r/3859/


Diffs
-

  /branches/11/funcs/func_jitterbuffer.c 419680 

Diff: https://reviewboard.asterisk.org/r/3860/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3861: datastores: Audit v12 ast_channel_datastore_remove usage.

2014-07-28 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3861/
---

(Updated July 28, 2014, 6:50 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419686


Repository: Asterisk


Description
---

Audit of v12 usage of ast_channel_datastore_remove() for datastore memory leaks.

* Fixed leaks in abstract_jb.

* Fixed leak in ast_channel_unsuppress().  Used by ARI mute control and 
res_mutestream.

* Fixed ref leak in ast_channel_suppress().  Used by ARI mute control and 
res_mutestream.

This is the additions for v12 over the v1.8 audit at 
https://reviewboard.asterisk.org/r/3859/ and v11 audit at 
https://reviewboard.asterisk.org/r/3860/


Diffs
-

  /branches/12/main/channel.c 419680 
  /branches/12/main/abstract_jb.c 419680 

Diff: https://reviewboard.asterisk.org/r/3861/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers

2014-07-25 Thread rmudgett 


 On July 24, 2014, 4:01 p.m., rmudgett wrote:
  You are affecting the lifetime of the continer node object without 
  protecting it.  It can now potentially be used outside the lifetime of its 
  container as part of the held object.  The container node and the continer 
  ponter in the container node would be stale.  This can happen when the 
  container is being destroyed at the same time as the object within the 
  container is being destroyed.  You would need to give the object a 
  reference to the container node and the container itself.  The reference to 
  the container prevents the nasty destruction race collision.  Yes, all 
  objects in the weak container would need to die before the container could 
  self destruct or the weak container can be explicitly emptied to get rid of 
  it earlier.  However, since they are expected to be used as global 
  containers that is not a problem at shutdown.
  
  It would be much simpler if the held object just contained a list of the 
  weak containers with a reference instead of the container's node object.  
  The ao2 object's clean up could simply use ao2_unlink() to remove itself 
  from the weak container.  The use of the container node to pull double duty 
  adds just too much coupling and complexity.
  
  Weak containers can only return an object after successfully getting its 
  reference (ao2_ref(obj, +1) returning a non-negative value).  If it fails 
  internal_ao2_traverse() must go on to the next object.
 
 George Joseph wrote:
 An object can't use a list of containers as this would mean the container 
 could only be in one object's list at a time.  I'd have to use a container of 
 containers which would be significant overhead.  That's why I went with node 
 since a node can only be associated with one object.  Having a reference to 
 the container makes sense though.  I think Corey suggested that a while back.

I think you are making an invalid assumption about ao2 containers.  An ao2 
container can hold the same object more than once.  The 
AO2_CONTAINER_ALLOC_OPT_DUPS_xxx options specify how a container handles 
duplicates.  Other than the AO2_CONTAINER_ALLOC_OPT_DUPS_ALLOW option, the 
container must be sorted.  ao2_unlink() only unlinks the object once if it is 
in the container.  If it is in the container several times, you must ao2_unlink 
it again.

An object can have the same container listed in its list of weak containers.  
Once for every time it is in that container.  I'm only talking about a simple 
list just like you are doing with the container nodes.  I'm not talking about 
an ao2 container to hold them as that would be a lot of overhead and overkill.

Object is a member of these weak reference containers:
channels
stasis-controlled
channels

The above object is in the channels container twice and once in the 
stasis-controlled container.  When the object is destroyed it unlinks itself 
from all containers listed.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3716/#review12860
---


On July 7, 2014, 11:43 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3716/
 ---
 
 (Updated July 7, 2014, 11:43 p.m.)
 
 
 Review request for Asterisk Developers, Corey Farrell and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Weak Reference Containers are hash containers that don't maintain references 
 to the objects they contain.
 Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't 
 decrease it.
 
 Sounds simple but because the container doesn't have a reference to the 
 object, it's entirely possible that the objects could be destroyed while 
 still in the container.  Therefore much of the work in this patch is making 
 sure that if the object is destroyed, it's properly cleaned out of any weak 
 reference containers that it may have been in.
 
 This patch is almost 6000 lines but half of it is units tests...functional, 
 performance, and stress.
 
 Richard:  I apologize in advance...I undid some of the refactor undos you had 
 me do earlier. :)
 
 
 Diffs
 -
 
   branches/12/utils/Makefile 418136 
   branches/12/tests/test_astobj2_weak.c PRE-CREATION 
   branches/12/tests/test_astobj2_torture.c PRE-CREATION 
   branches/12/tests/test_astobj2_thrash.c 418136 
   branches/12/tests/test_astobj2.c 418136 
   branches/12/main/astobj2_weak.c PRE-CREATION 
   branches/12/main/astobj2_rbtree.c 418136 
   branches/12/main/astobj2_private.h 418136 
   branches/12/main/astobj2_hash_private.h PRE-CREATION 
   branches/12/main/astobj2_hash.c 418136 
   branches/12/main/astobj2_container_private.h 418136 
   branches/12/main

Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers

2014-07-25 Thread rmudgett 


 On July 24, 2014, 4:01 p.m., rmudgett wrote:
  You are affecting the lifetime of the continer node object without 
  protecting it.  It can now potentially be used outside the lifetime of its 
  container as part of the held object.  The container node and the continer 
  ponter in the container node would be stale.  This can happen when the 
  container is being destroyed at the same time as the object within the 
  container is being destroyed.  You would need to give the object a 
  reference to the container node and the container itself.  The reference to 
  the container prevents the nasty destruction race collision.  Yes, all 
  objects in the weak container would need to die before the container could 
  self destruct or the weak container can be explicitly emptied to get rid of 
  it earlier.  However, since they are expected to be used as global 
  containers that is not a problem at shutdown.
  
  It would be much simpler if the held object just contained a list of the 
  weak containers with a reference instead of the container's node object.  
  The ao2 object's clean up could simply use ao2_unlink() to remove itself 
  from the weak container.  The use of the container node to pull double duty 
  adds just too much coupling and complexity.
  
  Weak containers can only return an object after successfully getting its 
  reference (ao2_ref(obj, +1) returning a non-negative value).  If it fails 
  internal_ao2_traverse() must go on to the next object.
 
 George Joseph wrote:
 An object can't use a list of containers as this would mean the container 
 could only be in one object's list at a time.  I'd have to use a container of 
 containers which would be significant overhead.  That's why I went with node 
 since a node can only be associated with one object.  Having a reference to 
 the container makes sense though.  I think Corey suggested that a while back.
 
 rmudgett wrote:
 I think you are making an invalid assumption about ao2 containers.  An 
 ao2 container can hold the same object more than once.  The 
 AO2_CONTAINER_ALLOC_OPT_DUPS_xxx options specify how a container handles 
 duplicates.  Other than the AO2_CONTAINER_ALLOC_OPT_DUPS_ALLOW option, the 
 container must be sorted.  ao2_unlink() only unlinks the object once if it is 
 in the container.  If it is in the container several times, you must 
 ao2_unlink it again.
 
 An object can have the same container listed in its list of weak 
 containers.  Once for every time it is in that container.  I'm only talking 
 about a simple list just like you are doing with the container nodes.  I'm 
 not talking about an ao2 container to hold them as that would be a lot of 
 overhead and overkill.
 
 Object is a member of these weak reference containers:
 channels
 stasis-controlled
 channels
 
 The above object is in the channels container twice and once in the 
 stasis-controlled container.  When the object is destroyed it unlinks itself 
 from all containers listed.
 
 George Joseph wrote:
 I understand how the containers work, it's linked lists that are the 
 issue.  The container needs a list entry structure to participate in a linked 
 list, no?  That means that that container can only be in 1 linked list that 
 uses that list entry at a time.  Hence it can't be in more than 1 object's 
 list at a time.

 
 George Joseph wrote:
 Let me rephrase...   the container can't be in multiple object's lists.


A list entry node needs to be allocated for every time it is put on an objects 
weak container membership list.

Allocate a struct like this for each entry added to the objects membership list 
when it is added.

struct weak_membership_node {
  AST_LIST_ENTRY(weak_membership_node) next;
  /*! Holds a reference to the weak container the object is in. */
  struct ao2_container *member_of;
}


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3716/#review12860
---


On July 7, 2014, 11:43 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3716/
 ---
 
 (Updated July 7, 2014, 11:43 p.m.)
 
 
 Review request for Asterisk Developers, Corey Farrell and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Weak Reference Containers are hash containers that don't maintain references 
 to the objects they contain.
 Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't 
 decrease it.
 
 Sounds simple but because the container doesn't have a reference to the 
 object, it's entirely possible that the objects could be destroyed while 
 still in the container.  Therefore much of the work in this patch

Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers

2014-07-25 Thread rmudgett 


 On July 24, 2014, 4:01 p.m., rmudgett wrote:
  You are affecting the lifetime of the continer node object without 
  protecting it.  It can now potentially be used outside the lifetime of its 
  container as part of the held object.  The container node and the continer 
  ponter in the container node would be stale.  This can happen when the 
  container is being destroyed at the same time as the object within the 
  container is being destroyed.  You would need to give the object a 
  reference to the container node and the container itself.  The reference to 
  the container prevents the nasty destruction race collision.  Yes, all 
  objects in the weak container would need to die before the container could 
  self destruct or the weak container can be explicitly emptied to get rid of 
  it earlier.  However, since they are expected to be used as global 
  containers that is not a problem at shutdown.
  
  It would be much simpler if the held object just contained a list of the 
  weak containers with a reference instead of the container's node object.  
  The ao2 object's clean up could simply use ao2_unlink() to remove itself 
  from the weak container.  The use of the container node to pull double duty 
  adds just too much coupling and complexity.
  
  Weak containers can only return an object after successfully getting its 
  reference (ao2_ref(obj, +1) returning a non-negative value).  If it fails 
  internal_ao2_traverse() must go on to the next object.
 
 George Joseph wrote:
 An object can't use a list of containers as this would mean the container 
 could only be in one object's list at a time.  I'd have to use a container of 
 containers which would be significant overhead.  That's why I went with node 
 since a node can only be associated with one object.  Having a reference to 
 the container makes sense though.  I think Corey suggested that a while back.
 
 rmudgett wrote:
 I think you are making an invalid assumption about ao2 containers.  An 
 ao2 container can hold the same object more than once.  The 
 AO2_CONTAINER_ALLOC_OPT_DUPS_xxx options specify how a container handles 
 duplicates.  Other than the AO2_CONTAINER_ALLOC_OPT_DUPS_ALLOW option, the 
 container must be sorted.  ao2_unlink() only unlinks the object once if it is 
 in the container.  If it is in the container several times, you must 
 ao2_unlink it again.
 
 An object can have the same container listed in its list of weak 
 containers.  Once for every time it is in that container.  I'm only talking 
 about a simple list just like you are doing with the container nodes.  I'm 
 not talking about an ao2 container to hold them as that would be a lot of 
 overhead and overkill.
 
 Object is a member of these weak reference containers:
 channels
 stasis-controlled
 channels
 
 The above object is in the channels container twice and once in the 
 stasis-controlled container.  When the object is destroyed it unlinks itself 
 from all containers listed.
 
 George Joseph wrote:
 I understand how the containers work, it's linked lists that are the 
 issue.  The container needs a list entry structure to participate in a linked 
 list, no?  That means that that container can only be in 1 linked list that 
 uses that list entry at a time.  Hence it can't be in more than 1 object's 
 list at a time.

 
 George Joseph wrote:
 Let me rephrase...   the container can't be in multiple object's lists.

 
 rmudgett wrote:
 A list entry node needs to be allocated for every time it is put on an 
 objects weak container membership list.
 
 Allocate a struct like this for each entry added to the objects 
 membership list when it is added.
 
 struct weak_membership_node {
   AST_LIST_ENTRY(weak_membership_node) next;
   /*! Holds a reference to the weak container the object is in. */
   struct ao2_container *member_of;
 }
 

 
 George Joseph wrote:
 How is this any better than just using the node?  Concurrency issues 
 aside, it's already allocated and it saves having to do a traverse on the 
 container just to find it and unlink it.

It does not increase module coupling/complexity by entangling itself with the 
lifetime of the container node object.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3716/#review12860
---


On July 7, 2014, 11:43 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3716/
 ---
 
 (Updated July 7, 2014, 11:43 p.m.)
 
 
 Review request for Asterisk Developers, Corey Farrell and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Weak Reference

Re: [asterisk-dev] [Code Review] 3852: Stasis App: Handle outbound masquerades

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3852/#review12878
---



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23238

function curly location



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23248

put blank line between variable declarations and code

break long line



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23242

This should not be necessary.  The zombie stasis channel (old_chan) is 
going to die.



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23240

I don't think this comment is needed.  Zombies drop like flies in winter. :)



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23246

This is one of those silly functions that can never fail but is defined to 
return a failure.  Also most callers don't care about the return code because 
it can never fail.

On the never gonna happen failure:
ast_datastore_free(datastore);



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23247

This should be a void function since even you don't care about the return 
value.



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23245

ast_datastore_free(datastore);



team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23241

RAII_VAR could be eliminated by saving send_msg_snapshot() ret value and 
calling ao2_cleanup().


- rmudgett


On July 25, 2014, 8:20 a.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3852/
 ---
 
 (Updated July 25, 2014, 8:20 a.m.)
 
 
 Review request for Asterisk Developers, Mark Michelson and rmudgett.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This handles the situation where a channel is masqueraded out of stasis to go 
 elsewhere. It ensures that the correct StasisEnd message is sent and performs 
 other cleanup necessary to prevent spurious messages from reaching Stasis 
 applications that aren't prepared for them.
 
 
 Diffs
 -
 
   team/group/ari-greedy-atxfer/res/res_stasis.c 419315 
 
 Diff: https://reviewboard.asterisk.org/r/3852/diff/
 
 
 Testing
 ---
 
 Used the AMI Bridge command to pull the channel currently in Stasis() into a 
 non-Stasis bridge and verified the messages coming back.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3852: Stasis App: Handle outbound masquerades

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3852/#review12880
---

Ship it!


Minor nit.


team/group/ari-greedy-atxfer/res/res_stasis.c
https://reviewboard.asterisk.org/r/3852/#comment23249

Probably should assert on this exit path because the datastore inexplicably 
dissapeared between finding it and removing it.

Or you could forget about the assert and either invert the test or make it 
unconditonal to eliminate a return path.



- rmudgett


On July 25, 2014, 4:30 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3852/
 ---
 
 (Updated July 25, 2014, 4:30 p.m.)
 
 
 Review request for Asterisk Developers, Mark Michelson and rmudgett.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This handles the situation where a channel is masqueraded out of stasis to go 
 elsewhere. It ensures that the correct StasisEnd message is sent and performs 
 other cleanup necessary to prevent spurious messages from reaching Stasis 
 applications that aren't prepared for them.
 
 
 Diffs
 -
 
   team/group/ari-greedy-atxfer/res/res_stasis.c 419315 
 
 Diff: https://reviewboard.asterisk.org/r/3852/diff/
 
 
 Testing
 ---
 
 Used the AMI Bridge command to pull the channel currently in Stasis() into a 
 non-Stasis bridge and verified the messages coming back.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

[asterisk-dev] [Code Review] 3859: datastores: Audit v1.8 ast_channel_datastore_remove usage.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3859/
---

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Audit of v1.8 usage of ast_channel_datastore_remove() usage for datastore 
memory leaks.

* Fixed leaks in app_speech_utils and func_frame_trace.

* Fixed app_speech_utils not locking the channel when accessing the channel 
datastore list.


Diffs
-

  /branches/1.8/funcs/func_frame_trace.c 419627 
  /branches/1.8/apps/app_speech_utils.c 419627 
  /branches/1.8/apps/app_queue.c 419627 

Diff: https://reviewboard.asterisk.org/r/3859/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3859: datastores: Audit v1.8 ast_channel_datastore_remove usage.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3859/
---

(Updated July 25, 2014, 5:23 p.m.)


Review request for Asterisk Developers.


Changes
---

Restore SpeechDestroy hanging up the channel if SpeechCreate has not already 
been called.  It is documented to be have this way with a suggested use of 
TryExec if that is not desired.


Repository: Asterisk


Description
---

Audit of v1.8 usage of ast_channel_datastore_remove() usage for datastore 
memory leaks.

* Fixed leaks in app_speech_utils and func_frame_trace.

* Fixed app_speech_utils not locking the channel when accessing the channel 
datastore list.


Diffs (updated)
-

  /branches/1.8/funcs/func_frame_trace.c 419627 
  /branches/1.8/apps/app_speech_utils.c 419627 
  /branches/1.8/apps/app_queue.c 419627 

Diff: https://reviewboard.asterisk.org/r/3859/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3844: features.c: Allow appliationmap to use Gosub.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3844/
---

(Updated July 25, 2014, 6:04 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419630


Bugs: AST-1391
https://issues.asterisk.org/jira/browse/AST-1391


Repository: Asterisk


Description
---

Using DYNAMIC_FEATURES with a Gosub application as the mapped application does 
not work.  It does not work because Gosub just pushes the current dialplan 
context, exten, and priority onto a stack and sets the specified Gosub 
location.  Gosub does not have a dialplan execution loop to run dialplan like 
Macro.

* Made the DYNAMIC_FEATURES application mapping feature call 
ast_app_exec_macro() and ast_app_exec_sub() for the Macro and Gosub 
applications respectively.

* Backported ast_app_exec_macro() and ast_app_exec_sub() from v11 to execute 
dialplan routines from the DYNAMIC_FEATURES application mapping feature.

NOTE: This issue does not affect v12+ because it already does what this patch 
implements.


Diffs
-

  /branches/1.8/main/features.c 419341 
  /branches/1.8/main/app.c 419341 
  /branches/1.8/include/asterisk/app.h 419341 
  /branches/1.8/apps/app_stack.c 419341 

Diff: https://reviewboard.asterisk.org/r/3844/diff/


Testing
---

Triggered DYNAMIC_FEATURES mapped to the following applications:
Playback
Gosub
Macro

The Playback played the file, the Gosub and Macro applications executed their 
respective dialplan sections.


Thanks,

rmudgett

-- 
_
-- 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

[asterisk-dev] [Code Review] 3860: datastores: Audit v11 ast_channel_datastore_remove usage.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3860/
---

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Audit of v11 usage of ast_channel_datastore_remove() usage for datastore memory 
leaks.

* Fixed leak in func_jitterbuffer.

This is the additions for v11 over the v1.8 audit at 
https://reviewboard.asterisk.org/r/3859/


Diffs
-

  /branches/11/funcs/func_jitterbuffer.c 419680 

Diff: https://reviewboard.asterisk.org/r/3860/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

[asterisk-dev] [Code Review] 3861: datastores: Audit v12 ast_channel_datastore_remove usage.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3861/
---

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Audit of v12 usage of ast_channel_datastore_remove() for datastore memory leaks.

* Fixed leaks in abstract_jb.

* Fixed leak in ast_channel_unsuppress().  Used by ARI mute control and 
res_mutestream.

* Fixed ref leak in ast_channel_suppress().  Used by ARI mute control and 
res_mutestream.

This is the additions for v12 over the v1.8 audit at 
https://reviewboard.asterisk.org/r/3859/ and v11 audit at 
https://reviewboard.asterisk.org/r/3860/


Diffs
-

  /branches/12/main/channel.c 419680 
  /branches/12/main/abstract_jb.c 419680 

Diff: https://reviewboard.asterisk.org/r/3861/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3860: datastores: Audit v11 ast_channel_datastore_remove usage.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3860/
---

(Updated July 25, 2014, 9:03 p.m.)


Review request for Asterisk Developers.


Repository: Asterisk


Description (updated)
---

Audit of v11 usage of ast_channel_datastore_remove() for datastore memory leaks.

* Fixed leak in func_jitterbuffer.

This is the additions for v11 over the v1.8 audit at 
https://reviewboard.asterisk.org/r/3859/


Diffs
-

  /branches/11/funcs/func_jitterbuffer.c 419680 

Diff: https://reviewboard.asterisk.org/r/3860/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3859: datastores: Audit v1.8 ast_channel_datastore_remove usage.

2014-07-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3859/
---

(Updated July 25, 2014, 9:04 p.m.)


Review request for Asterisk Developers.


Repository: Asterisk


Description (updated)
---

Audit of v1.8 usage of ast_channel_datastore_remove() for datastore memory 
leaks.

* Fixed leaks in app_speech_utils and func_frame_trace.

* Fixed app_speech_utils not locking the channel when accessing the channel 
datastore list.


Diffs
-

  /branches/1.8/funcs/func_frame_trace.c 419627 
  /branches/1.8/apps/app_speech_utils.c 419627 
  /branches/1.8/apps/app_queue.c 419627 

Diff: https://reviewboard.asterisk.org/r/3859/diff/


Testing
---

Code inspection and compiler.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers

2014-07-24 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3716/#review12860
---


You are affecting the lifetime of the continer node object without protecting 
it.  It can now potentially be used outside the lifetime of its container as 
part of the held object.  The container node and the continer ponter in the 
container node would be stale.  This can happen when the container is being 
destroyed at the same time as the object within the container is being 
destroyed.  You would need to give the object a reference to the container node 
and the container itself.  The reference to the container prevents the nasty 
destruction race collision.  Yes, all objects in the weak container would need 
to die before the container could self destruct or the weak container can be 
explicitly emptied to get rid of it earlier.  However, since they are expected 
to be used as global containers that is not a problem at shutdown.

It would be much simpler if the held object just contained a list of the weak 
containers with a reference instead of the container's node object.  The ao2 
object's clean up could simply use ao2_unlink() to remove itself from the weak 
container.  The use of the container node to pull double duty adds just too 
much coupling and complexity.

Weak containers can only return an object after successfully getting its 
reference (ao2_ref(obj, +1) returning a non-negative value).  If it fails 
internal_ao2_traverse() must go on to the next object.


branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23222

You need to separate the magic number bits from the state bits.  The magic 
number bits should never change.

As it is you could have a thread check an object for validation and not see 
a valid value because it was in the process of being updated.




branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23223

Keeping the magic number at the end of the struct provided some protection 
from underwrites performed on the user data.



branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23230

This function's purpose is a falacy.  It's result is valid only for the 
instant before it returns.  Once it releases the spin lock the object could be 
destroyed by another thread.  Any decisions made by code after it returns are 
suspect if the object is returned as valid.



branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23231

Destroy the object's user data AFTER it has been removed from all weak 
containers.  Weak containers can still access the user data with the container 
callbacks during traversals.  __is_ao2_object_healthy() does not prevent the 
access.



branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23227

Use !AST_LIST_EMPTY()



branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23229

AST_LIST_TRAVERSE_SAFE_BEGIN does not provide safety from other threads.  
It provides safety from actions within the loop affecting the list's current 
node only.

Also using the node-my_container pointer here is unsafe.  That pointer 
does not poses a reference to the container and the container node can out live 
the container at this point.




branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3716/#comment23225

This comment belongs a couple lines up where the state is assigned AO2_DEAD.



branches/12/main/astobj2_weak.c
https://reviewboard.asterisk.org/r/3716/#comment23228

The weak container needs its own destructor function to mitigate the race 
if the container destructor happens at the same time as an object in the 
container is being destroyed.

Either it needs to assert that the container is empty.
or
1) It needs to unlink all objects in the container as normal.
2) It needs to wait for any pending object destructor collision to complete.
3) It needs to check that there are no more nodes remaining for the normal 
reference leak check.  This can probably be done at the same time as waiting 
for the pending destructor collision nodes to go away.



- rmudgett


On July 7, 2014, 11:43 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3716/
 ---
 
 (Updated July 7, 2014, 11:43 p.m.)
 
 
 Review request for Asterisk Developers, Corey Farrell and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Weak Reference Containers are hash containers that don't maintain references 
 to the objects they contain.
 Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't 
 decrease it.
 
 Sounds simple

Re: [asterisk-dev] [Code Review] 3840: bridge: Move bridge destroy CLI command to devmode and add all to bridge kick CLI command

2014-07-24 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3840/#review12864
---



/branches/12/main/bridge.c
https://reviewboard.asterisk.org/r/3840/#comment23232

This RAII_VAR can easily be eliminated as well.  There are only two exit 
points that need to cleanup.



/branches/12/main/bridge.c
https://reviewboard.asterisk.org/r/3840/#comment23233

...name all..
...name then all...



/branches/12/main/bridge.c
https://reviewboard.asterisk.org/r/3840/#comment23234

Maybe add a:
Kicking everything from bridge '%s'.
message.


- rmudgett


On July 23, 2014, 4:53 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3840/
 ---
 
 (Updated July 23, 2014, 4:53 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The bridge destroy CLI command is dangerous for mere mortals. It can send 
 bridges into an unusable state when the owner of them did not expect it to 
 happen. Its very existence instills paranoia upon the users of bridges since 
 they must be aware that the very bridge they created may need to be recreated 
 at any time. I think this is unreasonable so I've moved it to be a devmode 
 only CLI command since it can potentially be useful when working with things. 
 In its normal place I have added all as a valid option to the bridge kick 
 CLI command. When invoked all channels will be kicked out of the bridge 
 instead of one specific one.
 
 
 Diffs
 -
 
   /branches/12/main/bridge.c 419126 
 
 Diff: https://reviewboard.asterisk.org/r/3840/diff/
 
 
 Testing
 ---
 
 Executed bridge kick all and confirmed all channels were kicked out.
 
 
 Thanks,
 
 Joshua Colp
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3840: bridge: Move bridge destroy CLI command to devmode and add all to bridge kick CLI command

2014-07-24 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3840/#review12866
---



/branches/12/main/bridge.c
https://reviewboard.asterisk.org/r/3840/#comment23235

This was the RAII_VAR I was pointing out.


- rmudgett


On July 24, 2014, 4:51 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3840/
 ---
 
 (Updated July 24, 2014, 4:51 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The bridge destroy CLI command is dangerous for mere mortals. It can send 
 bridges into an unusable state when the owner of them did not expect it to 
 happen. Its very existence instills paranoia upon the users of bridges since 
 they must be aware that the very bridge they created may need to be recreated 
 at any time. I think this is unreasonable so I've moved it to be a devmode 
 only CLI command since it can potentially be useful when working with things. 
 In its normal place I have added all as a valid option to the bridge kick 
 CLI command. When invoked all channels will be kicked out of the bridge 
 instead of one specific one.
 
 
 Diffs
 -
 
   /branches/12/main/bridge.c 419341 
 
 Diff: https://reviewboard.asterisk.org/r/3840/diff/
 
 
 Testing
 ---
 
 Executed bridge kick all and confirmed all channels were kicked out.
 
 
 Thanks,
 
 Joshua Colp
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3840: bridge: Move bridge destroy CLI command to devmode and add all to bridge kick CLI command

2014-07-24 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3840/#review12867
---

Ship it!


Ship It!

- rmudgett


On July 24, 2014, 4:56 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3840/
 ---
 
 (Updated July 24, 2014, 4:56 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The bridge destroy CLI command is dangerous for mere mortals. It can send 
 bridges into an unusable state when the owner of them did not expect it to 
 happen. Its very existence instills paranoia upon the users of bridges since 
 they must be aware that the very bridge they created may need to be recreated 
 at any time. I think this is unreasonable so I've moved it to be a devmode 
 only CLI command since it can potentially be useful when working with things. 
 In its normal place I have added all as a valid option to the bridge kick 
 CLI command. When invoked all channels will be kicked out of the bridge 
 instead of one specific one.
 
 
 Diffs
 -
 
   /branches/12/main/bridge.c 419341 
 
 Diff: https://reviewboard.asterisk.org/r/3840/diff/
 
 
 Testing
 ---
 
 Executed bridge kick all and confirmed all channels were kicked out.
 
 
 Thanks,
 
 Joshua Colp
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3601: accountcode: Slightly change accountcode propagation.

2014-07-24 Thread rmudgett 
 channel.  The acccountcode
on the calling channel forces itself on the outgoing channel.  This is the
same as previous behavior.

* Set the accountcode and peeraccount code on the calling channel and let
the channel driver supply or not the acccountcode for the outgoing
channel.  The outgoing channel's accountcode and peeraccount got the
calling channel's peeraccount and accountcode values respectively.

* Let dialplan set accountcodes on channels and performed a DTMF attended
transfer to create a three party bridge.  When the transferrer channel
left the three party bridge, the remaining two channels got the
peeraccount updated to the other party.  Unfortunately, you only see the
updated peeraccount values in the CEL log when the channels leave the
bridge.

Throughout each of these tests, the CEL log had the expected peeraccount
value.  Some interpolation is needed in the CEL log for complicated
accountcode manipulation because there aren't enough events to indicate
when the account codes change.  Note the peeraccount value is meaningless
if the bridge does not contain two parties.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3832: testsuite: Accountcode propagation.

2014-07-24 Thread rmudgett 
/cdr_accountcode/test-config.yaml 
5295 
  
/asterisk/trunk/tests/cdr/cdr_properties/blind-transfer-accountcode/test-config.yaml
 5295 
  /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/test-config.yaml 
5295 
  
/asterisk/trunk/tests/cdr/cdr_manipulation/console_fork_before_dial/test-config.yaml
 5295 
  
/asterisk/trunk/tests/cdr/cdr_manipulation/console_fork_after_busy_forward/test-config.yaml
 5295 
  /asterisk/trunk/tests/cdr/cdr_manipulation/cdr_fork_end_time/test-config.yaml 
5295 
  /asterisk/trunk/lib/python/asterisk/test_case.py 5295 
  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 5295 

Diff: https://reviewboard.asterisk.org/r/3832/diff/


Testing
---

All accountcode tagged tests pass.


Thanks,

rmudgett

-- 
_
-- 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

[asterisk-dev] [Code Review] 3844: features.c: Allow appliationmap to use Gosub.

2014-07-23 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3844/
---

Review request for Asterisk Developers.


Bugs: AST-1391
https://issues.asterisk.org/jira/browse/AST-1391


Repository: Asterisk


Description
---

Using DYNAMIC_FEATURES with a Gosub application as the mapped application does 
not work.  It does not work because Gosub just pushes the current dialplan 
context, exten, and priority onto a stack and sets the specified Gosub 
location.  Gosub does not have a dialplan execution loop to run dialplan like 
Macro.

* Made the DYNAMIC_FEATURES application mapping feature call 
ast_app_exec_macro() and ast_app_exec_sub() for the Macro and Gosub 
applications respectively.

* Backported ast_app_exec_macro() and ast_app_exec_sub() from v11 to execute 
dialplan routines from the DYNAMIC_FEATURES application mapping feature.

NOTE: This issue does not affect v12+ because it already does what this patch 
implements.


Diffs
-

  /branches/1.8/main/features.c 419341 
  /branches/1.8/main/app.c 419341 
  /branches/1.8/include/asterisk/app.h 419341 
  /branches/1.8/apps/app_stack.c 419341 

Diff: https://reviewboard.asterisk.org/r/3844/diff/


Testing
---

Triggered DYNAMIC_FEATURES mapped to the following applications:
Playback
Gosub
Macro

The Playback played the file, the Gosub and Macro applications executed their 
respective dialplan sections.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3759/#review12807
---



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23135

Where are file, line, and func coming from?  They don't seem to be passed 
in to sip_alloc.



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23137

Something seems off here.  The ao2_t_ref() calls return ints and are 
parameters to AST_SCHED_REPLACE_UNREF().  Didn't registry_unref() return a 
pointer?




/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23138

Idem



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23139

Idem



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23140

You should get a iterator ref for ast_sched_add() before calling it and 
unref if it fails to be added.  Otherwise, you run the risk of iterator being 
stale when ast_sched_add succeeds.


- rmudgett


On July 21, 2014, 10: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, 10: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

Re: [asterisk-dev] [Code Review] 3813: codec_speex: Fix trashing normal static frame for AST_FRAME_CNG.

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3813/
---

(Updated July 22, 2014, 12:10 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419206


Repository: Asterisk


Description
---

Made use a local static frame to generate the AST_FRAME_CNG frame when silence 
starts.

I don't think the handling of the AST_FRAME_CNG has ever really worked because 
there doesn't seem to be any consumers of it.


Diffs
-

  /team/group/media_formats-reviewed-trunk/codecs/codec_speex.c 418785 

Diff: https://reviewboard.asterisk.org/r/3813/diff/


Testing
---

It compiles.  I don't have anything that will consume/produce speex.


Thanks,

rmudgett

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3728: ARI: Add missing transfer information

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3728/#review12808
---

Ship it!


Ship It!

- rmudgett


On July 18, 2014, 12:42 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3728/
 ---
 
 (Updated July 18, 2014, 12:42 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds a new field to the ast_attended_transfer_type() and a new subtype 
 AST_ATTENDED_TRANSFER_DEST_LOCAL_APP to describe attended transfers where a 
 local channel is used to connect a dialplan application to a bridge in cases 
 where a single remote party can not be moved directly into the application. 
 Previously, the local channel half being pushed into the bridge in place of a 
 transferer leg was not conveyed in the message.
 
 
 Diffs
 -
 
   branches/12/rest-api/api-docs/events.json 418910 
   branches/12/res/ari/ari_model_validators.c 418910 
   branches/12/res/ari/ari_model_validators.h 418910 
   branches/12/main/stasis_bridges.c 418910 
   branches/12/main/cel.c 418910 
   branches/12/main/bridge.c 418910 
   branches/12/include/asterisk/stasis_bridges.h 418910 
   branches/12/apps/app_queue.c 418910 
 
 Diff: https://reviewboard.asterisk.org/r/3728/diff/
 
 
 Testing
 ---
 
 Tested as part of the complete solution to ASTERISK-23941.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3731: Stasis: Prevent non-stasis channels from entering stasis bridges

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3731/#review12809
---



team/rmudgett/stasis_linkedids/res/stasis/app.c
https://reviewboard.asterisk.org/r/3731/#comment23141

This is not NULL tollerant.
Use ast_channel_cleanup() instead.


- rmudgett


On July 18, 2014, 12:58 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3731/
 ---
 
 (Updated July 18, 2014, 12:58 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This intercepts channels attempting to enter stasis-controlled bridges that 
 are not themselves controlled by stasis and routes them through Stasis() to 
 be controlled by the Stasis application that controls the channels they are 
 replacing.
 
 
 Diffs
 -
 
   team/rmudgett/stasis_linkedids/rest-api/api-docs/events.json 418962 
   team/rmudgett/stasis_linkedids/res/stasis/stasis_bridge.c 418962 
   team/rmudgett/stasis_linkedids/res/stasis/control.c 418962 
   team/rmudgett/stasis_linkedids/res/stasis/control.h 418962 
   team/rmudgett/stasis_linkedids/res/stasis/app.c 418962 
   team/rmudgett/stasis_linkedids/res/stasis/app.h 418962 
   team/rmudgett/stasis_linkedids/res/res_stasis.c 418962 
   team/rmudgett/stasis_linkedids/res/ari/ari_model_validators.c 418962 
   team/rmudgett/stasis_linkedids/res/ari/ari_model_validators.h 418962 
 
 Diff: https://reviewboard.asterisk.org/r/3731/diff/
 
 
 Testing
 ---
 
 Tested against the updated ARI attended transfer test in 
 https://reviewboard.asterisk.org/r/3732/
 
 
 File Attachments
 
 
 Collation of thiis patch and dependency patches.
   
 https://reviewboard.asterisk.org/media/uploaded/files/2014/07/10/3331f67f-8c7d-486f-bdcf-bf9a01aa288d__ari_atxfer.diff
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3729: Stasis: Allow prestart actions to be queued

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3729/#review12811
---

Ship it!



branches/12/res/stasis/control.c
https://reviewboard.asterisk.org/r/3729/#comment23145

The use of ao2_container_count() assumes nobody else can get access to the 
container to add more commands.  This isn't theoretically true since the 
container is still obtainable from the channel datastore.  It's a good thing 
nothing cares about this return value.  The way you had it counting each 
command executed before will always give you the number of commands actually 
executed.


- rmudgett


On July 21, 2014, 4:55 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3729/
 ---
 
 (Updated July 21, 2014, 4:55 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This allows commands to be queued in a channel datastore to be executed upon 
 a channel entering Stasis(). This functionality is only available from 
 components of res_stasis.so. This functionality is required to get a 
 redirected channel back into the bridge for which it was destined due to the 
 attended transfer.
 
 
 Diffs
 -
 
   branches/12/res/stasis/control.c 419109 
   branches/12/res/stasis/control.h 419109 
   branches/12/res/stasis/command.c 419109 
   branches/12/res/stasis/command.h 419109 
   branches/12/res/res_stasis.c 419109 
 
 Diff: https://reviewboard.asterisk.org/r/3729/diff/
 
 
 Testing
 ---
 
 Tested as part of the complete solution to ASTERISK-23941.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3731: Stasis: Prevent non-stasis channels from entering stasis bridges

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3731/#review12813
---

Ship it!


Ship It!

- rmudgett


On July 22, 2014, 1:08 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3731/
 ---
 
 (Updated July 22, 2014, 1:08 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This intercepts channels attempting to enter stasis-controlled bridges that 
 are not themselves controlled by stasis and routes them through Stasis() to 
 be controlled by the Stasis application that controls the channels they are 
 replacing.
 
 
 Diffs
 -
 
   trunk/rest-api/api-docs/events.json 419221 
   trunk/res/stasis/stasis_bridge.c 419221 
   trunk/res/stasis/control.c 419221 
   trunk/res/stasis/control.h 419221 
   trunk/res/stasis/app.c 419221 
   trunk/res/stasis/app.h 419221 
   trunk/res/res_stasis.c 419221 
   trunk/res/ari/ari_model_validators.c 419221 
   trunk/res/ari/ari_model_validators.h 419221 
 
 Diff: https://reviewboard.asterisk.org/r/3731/diff/
 
 
 Testing
 ---
 
 Tested against the updated ARI attended transfer test in 
 https://reviewboard.asterisk.org/r/3732/
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3836: app_bridgewait: Remove race condition where bridge may be dissolved when trying to join

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3836/#review12816
---



/branches/12/apps/app_bridgewait.c
https://reviewboard.asterisk.org/r/3836/#comment23149

Should check if the bridge in the wrapper is dissolved before returning the 
wrapper.  If it is dissolved, the wrapper needs to be unlinked and a new bridge 
created.

Some joker could CLI bridge destroy id the BridgeWait holding bridge.  
This would prevent new channels from entering the BridgeWait holding bridge 
until all channels that entered the bridging system in that holding bridge 
hangup.  It could take awhile if a channel was moved to a normal bridge.


- rmudgett


On July 22, 2014, 10:14 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3836/
 ---
 
 (Updated July 22, 2014, 10:14 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23987
 https://issues.asterisk.org/jira/browse/ASTERISK-23987
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The BridgeWait application currently creates bridges with the dissolve on 
 empty flag set. This causes the bridge to be dissolved when the last channel 
 leaves it. This introduces a race condition where another channel may be 
 trying to join during this, causing it to fail. Since the lifetime of the 
 bridge is already associated with the bridge wrapper the bridge does not need 
 the dissolve on empty flag set. When the last reference goes away the bridge 
 is destroyed. This ensures that as long as anything has a reference to the 
 bridge wrapper the bridge is valid and can be joined.
 
 
 Diffs
 -
 
   /branches/12/apps/app_bridgewait.c 418809 
 
 Diff: https://reviewboard.asterisk.org/r/3836/diff/
 
 
 Testing
 ---
 
 Ran tests and confirmed no regressions.
 
 
 Thanks,
 
 Joshua Colp
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3832: testsuite: Accountcode propagation.

2014-07-22 Thread rmudgett 


 On July 22, 2014, 12:41 p.m., Matt Jordan wrote:
  /asterisk/trunk/tests/pbx/accountcode/dial_none/configs/ast1/extensions.conf,
   lines 7-10
  https://reviewboard.asterisk.org/r/3832/diff/1/?file=64930#file64930line7
 
  Having to wait 5 seconds for this test to end when it has essentially 
  completed is usually something we avoid.
  
  There is another route you can take, however:
  (1) Drop the channel into Echo after answering it
  (2) Use the channel hangup module to hangup the channel on the Newexten 
  event:
  
  modules:
  -
  config-section: hangup-channel
  typename: 'pluggable_modules.AMIChannelHangup'
  
  hangup-channel:
  id: '1'
  conditions:
  match:
  Event: 'Newexten'
  Channel: 'PJSIP/bob-.*'
  Application: Echo
  count: '1'
  
  This also supports a 'delay' parameter in case the hangup *does* need 
  to wait a second or two.
  
  This finding would extend to the other tests that use the same pattern.

How is the 'delay' parameter any different than the Wait?  The Wait is to allow 
the answer to percolate back up the chain and to allow the channels to enter 
the bridge before hanging up.  Otherwise, the channels may not enter the bridge 
at all.


 On July 22, 2014, 12:41 p.m., Matt Jordan wrote:
  /asterisk/trunk/tests/pbx/accountcode/dial_none/test-config.yaml, line 61
  https://reviewboard.asterisk.org/r/3832/diff/1/?file=64932#file64932line61
 
  I don't think you need the dependency on func_channel for this test.

With so many tests it was getting difficult to keep those dependencies 
straight. :)


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3832/#review12810
---


On July 21, 2014, 4:49 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3832/
 ---
 
 (Updated July 21, 2014, 4:49 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: AFS-65
 https://issues.asterisk.org/jira/browse/AFS-65
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 New tests to check accountcode propagation with the new 
 accouncode/peeracccount interaction.
 
 * Made pluggable_modules.py Originator class and test_case.py
 SimpleTestCase class call origination allow specifying the accountcode.
 
 * Fix tests/cdr/sqlite3 to work with the new accountcode propagation rules.
 
 * Add accountcode tag to existing tests doing something with accountcode.
 
 Testsuite tests to go with https://reviewboard.asterisk.org/r/3601/
 
 
 Diffs
 -
 
   /asterisk/trunk/tests/pbx/tests.yaml 5288 
   /asterisk/trunk/tests/pbx/accountcode/tests.yaml PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/test-config.yaml 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/configs/ast1/queues.conf
  PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/configs/ast1/pjsip.conf
  PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/configs/ast1/extensions.conf
  PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/queue_peer_none/test-config.yaml 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_peer_none/configs/ast1/queues.conf
  PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_peer_none/configs/ast1/pjsip.conf 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_peer_none/configs/ast1/extensions.conf
  PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/queue_none/test-config.yaml 
 PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/queue_none/configs/ast1/queues.conf 
 PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/queue_none/configs/ast1/pjsip.conf 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/queue_none/configs/ast1/extensions.conf 
 PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/local_originate/test-config.yaml 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/local_originate/configs/ast1/extensions.conf
  PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/local_crossover_back/test-config.yaml 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/local_crossover_back/configs/ast1/extensions.conf
  PRE-CREATION 
   /asterisk/trunk/tests/pbx/accountcode/local_crossover/test-config.yaml 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/local_crossover/configs/ast1/extensions.conf
  PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/dial_straight_override/test-config.yaml 
 PRE-CREATION 
   
 /asterisk/trunk/tests/pbx/accountcode/dial_straight_override

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2

2014-07-22 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3759/#review12826
---



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23168

Try -1 instead :)


- rmudgett


On July 22, 2014, 5:01 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3759/
 ---
 
 (Updated July 22, 2014, 5:01 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

[asterisk-dev] [Code Review] 3832: testsuite: Accountcode propagation.

2014-07-21 Thread rmudgett 
 
  /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/test-config.yaml 
5288 
  
/asterisk/trunk/tests/cdr/cdr_manipulation/console_fork_before_dial/test-config.yaml
 5288 
  
/asterisk/trunk/tests/cdr/cdr_manipulation/console_fork_after_busy_forward/test-config.yaml
 5288 
  /asterisk/trunk/tests/cdr/cdr_manipulation/cdr_fork_end_time/test-config.yaml 
5288 
  /asterisk/trunk/lib/python/asterisk/test_case.py 5288 
  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 5288 

Diff: https://reviewboard.asterisk.org/r/3832/diff/


Testing
---

All accountcode tagged tests pass.


Thanks,

rmudgett

-- 
_
-- 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

<    1   2   3   4   5   6   7   8   9   >