Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-10-14 Thread Matt Jordan

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



/trunk/main/presencestate.c
https://reviewboard.asterisk.org/r/4050/#comment24030

So, I'm not sure this is the behaviour that we would want.

If a channel driver provides presence information, then the presence 
information provided by the channel driver supercedes any custom presence 
provider. That is:

exten = hint,SIP/aliceCustomPresence:alice

Will *always* use the presence provided by SIP/alice, instead of the custom 
presence provider. That feels like a loss of functionality.

Prior to this patch, we would only use the presence information provided by 
a single custom presence provider, since presence information could only come 
from custom presence providers. If the channel drivers provide that as well, 
there probably needs to be some mechanism to aggregate that together.


- Matt Jordan


On Oct. 5, 2014, 11:18 p.m., gareth wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4050/
 ---
 
 (Updated Oct. 5, 2014, 11:18 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24363
 https://issues.asterisk.org/jira/browse/ASTERISK-24363
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch adds the ability for channel drivers to supply presence 
 information in a similar manner to device state.
 
 eg: exten = XXX,hint,,Technology/Resource
 
 
 Diffs
 -
 
   /trunk/main/presencestate.c 424055 
   /trunk/main/channel.c 424055 
   /trunk/include/asterisk/channel.h 424055 
 
 Diff: https://reviewboard.asterisk.org/r/4050/diff/
 
 
 Testing
 ---
 
 Code is originally written as part of ASTERISK-13145 which has undergone 
 extensive testing.
 
 
 Thanks,
 
 gareth
 


-- 
_
-- 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] 4050: Add ability for Channel Drivers to provide Presence State information

2014-10-14 Thread Olle E. Johansson

On 14 Oct 2014, at 15:51, Matt Jordan reviewbo...@asterisk.org wrote:

 exten = hint,SIP/aliceCustomPresence:alice
 
 Will *always* use the presence provided by SIP/alice, instead of the custom 
 presence provider. That feels like a loss of functionality.
 
 Prior to this patch, we would only use the presence information provided by a 
 single custom presence provider, since presence information could only come 
 from custom presence providers. If the channel drivers provide that as well, 
 there probably needs to be some mechanism to aggregate that together.
 
THis looks insane to me, but I must surely have missed something. Have we 
discussed the definition of presence ?
In the old days we had extension states and device states - for me that's not 
presence, but can be input to personal presence.
What is this stuff?

Curious. But maybe too late to jump into the discussion :-)

/O-- 
_
-- 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] 3948: Asterisk does not respect outbound proxy when sending qualify requests

2014-10-14 Thread Matt Jordan


 On Aug. 29, 2014, 6:04 p.m., Damian Ivereigh wrote:
  Thanks for all that info Matt. In answer to the question how should 
  outboundproxy behave, perhaps it might be useful to detail my setup. I 
  have a number of Asterisk servers on an internal network with a kamailio 
  server and a media proxy facing the internet. My goal was to harden the 
  kamailio server and allow the asterisk servers to be less secure and allow 
  each asterisk to define it's own peers. I want as close as possible for 
  each Asterisk server to appear to the outside world as if they are 
  externally connected (no NAT stuff), yet actually put everything through 
  kamailio and the media proxy.
  
  So the obvious solution was to use outboundproxy to get asterisk to send 
  its outgoing invites and registrations through the kamailio server which 
  would mangle them so that everything appeared to come from the external 
  server. However things fell apart when asterisk tried to send qualify 
  requests direct (which the firewall blocked). Hence this fix. I really 
  cannot see a situation where one would use an outboundproxy and then want 
  to send the qualify requests directly.
 
 wdoekes wrote:
 Re: mem leak: https://reviewboard.asterisk.org/r/4016/
 
 As for the how should the outboundproxy behave. I agree that this
 addition makes sense. But I'd like to hear someone else who uses
 obproxy to chime in too before giving this the go-ahead.

So, I haven't been able to drum up any feedback from those who use 
outboundproxy.

In the absence of any feedback, the only suggestion I have is that we apply the 
patch and hope for the best. I'd ask that Damian be willing to help respond to 
any outbound proxy issues that may arise as a result of this patch (which I 
would say has minimal risk, given the confidence that the current behavior is 
broken).


- Matt


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


On Aug. 25, 2014, 5:04 p.m., Damian Ivereigh wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3948/
 ---
 
 (Updated Aug. 25, 2014, 5:04 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24063
 https://issues.asterisk.org/jira/browse/ASTERISK-24063
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The outboundproxy setting is ignored when sending the qualify packets 
 (OPTIONS). This means that if an asterisk server is unable to send the packet 
 directly to a peer, it is unable to qualify any non inbound registered peer 
 (e.g. a peer SIP Trunk). This problem is found on asterisk-11.6-cert4 (and 
 many others)
 
 It has been pointed out (thanks Walter Doekes), that the p-outboundproxy may 
 not be freed at the end which would create a memory leak.   
 
 
 Diffs
 -
 
   certified/tags/11.6-cert4/channels/chan_sip.c 422052 
 
 Diff: https://reviewboard.asterisk.org/r/3948/diff/
 
 
 Testing
 ---
 
 Have run this change in production for many months, however the possible 
 memory leak issue needs to be verified.
 
 
 Thanks,
 
 Damian Ivereigh
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4075: parking/tests: Running res_parking unit tests would cause assertions and possibly a crash due to attempting to play MOH on a channel with no formats

2014-10-14 Thread rmudgett

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



/branches/13/res/parking/parking_tests.c
https://reviewboard.asterisk.org/r/4075/#comment24031

Unlock alice before release.


- rmudgett


On Oct. 13, 2014, 5:45 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4075/
 ---
 
 (Updated Oct. 13, 2014, 5:45 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Bugs: ASTERISK-24413
 https://issues.asterisk.org/jira/browse/ASTERISK-24413
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch simply follows the suggested fix of specifying the format for test 
 channels in the same manner as was done for the CDR unit tests.
 
 
 Diffs
 -
 
   /branches/13/res/parking/parking_tests.c 425404 
 
 Diff: https://reviewboard.asterisk.org/r/4075/diff/
 
 
 Testing
 ---
 
 Ran tests prior to patch and got assertions.  Assertions no longer occurred 
 with the patch in place.
 
 
 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] 4077: Test Suite: Unregister registered outbound registrations

2014-10-14 Thread jbigelow

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

Review request for Asterisk Developers.


Repository: testsuite


Description
---

This tests unregistering outbound registrations that are registered using the 
PJSIPUnregister AMI action. It ensures that Asterisk sends a REGISTER message 
with an Expires header of '0' upon executing the PJSIPUnregister AMI action and 
that AMI Registry events are present with a status of Unregistered. The test 
uses all combinations of IPv4/IPv6 and UDP/TCP.

Upon an outbound registration registering, the PJSIPUnregister action is 
executed to unregister it. SIPp is used as the registrar to respond when 
Asterisk registers and unregisters where the Expires header is checked. 

Note: This test currently fails due to the following Asterisk bugs:
* https://issues.asterisk.org/jira/browse/ASTERISK-24411
* https://issues.asterisk.org/jira/browse/ASTERISK-24414

The SIPp scenario contains a pause at the end to work around ASTERISK-24414 for 
the time being. The 'expectedResult' property is set within the YAML file to 
expect the test to fail (and thus pass) while ASTERISK-24411 is unresolved 
however there appears to be a bug in testsuite as it's still marked as failed.


Diffs
-

  
/asterisk/trunk/tests/channels/pjsip/registration/outbound/unregister/unauthed/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/registration/outbound/unregister/unauthed/sipp/unregister.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/registration/outbound/unregister/unauthed/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/registration/outbound/unregister/tests.yaml
 PRE-CREATION 
  /asterisk/trunk/tests/channels/pjsip/registration/outbound/tests.yaml 5726 

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


Testing
---

* Ran test in a shell loop 300 times without failure (with patch for Asterisk 
applied from ASTERISK-24411 and work around for ASTERISK-24414)
* Ran test several times without patch applied from ASTERISK-24411 which 
constantly failed as expected.
* Ran test several times without work around for ASTERISK-24414 which 
frequently failed as expected.


Thanks,

jbigelow

-- 
_
-- 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] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread George Joseph

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

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
while doing an ast_category_delete in an ast_category_browse loop.  Apparently 
this never worked but was also never tested.  I removed the test, added 2 notes 
to config.h indicating that it's not supported and added a few lines of code to 
ast_category_delete to prevent the SEGV should someone attempt it in the 
future.  


Diffs
-

  branches/12/tests/test_config.c 425404 
  branches/12/main/config.c 425404 
  branches/12/include/asterisk/config.h 425404 

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


Testing
---

The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
tests still passes.


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] 4048: res_fax: Fix fax handler module reference leak

2014-10-14 Thread Corey Farrell

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

(Updated Oct. 14, 2014, 11:16 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 425405


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


Repository: Asterisk


Description
---

fax_session_reserve adds a reference to the fax driver's module, then 
fax_session_new adds another reference, but when the session is freed it only 
removes a single reference to the module.

This fixes it by unreferencing the module from fax_session_new when it is based 
on a reserved session.


Diffs
-

  /branches/1.8/res/res_fax.c 424175 

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


Testing
---

In testsuite against 11 verified that res_fax_spandsp now unloads in tests that 
previously leaked references to the module.


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] 4066: stasis_channels.c: Resolve unfinished Dials when doing masquerades (Part 2)

2014-10-14 Thread rmudgett

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

(Updated Oct. 14, 2014, 11:24 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 425430


Bugs: ASTERISK-24237 and ASTERISK-24394
https://issues.asterisk.org/jira/browse/ASTERISK-24237
https://issues.asterisk.org/jira/browse/ASTERISK-24394


Repository: Asterisk


Description
---

Masquerades into and out of channels that are involved in a dial operation 
don't create the expected dial end event.  The missing dial end event goes 
against the model for things like CDRs and generating Dial end manager actions 
and such.


Diffs
-

  /branches/12/main/stasis_channels.c 424943 

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


Testing
---

There are four cases:
1) A channel masquerades into the caller channel.  The case happens when 
performing a blonde transfer using the channel driver's protocol.
2) A channel masquerades into a callee channel.  The case happens when 
performing a directed call pickup.
3) The caller channel masquerades out of dial.  The case happens when using the 
Bridge application on the caller channel.
4) A callee channel masquerades out of dial.  The case happens when using the 
Bridge application on a peer channel.

The four cases are now handled as expected from the dial events perspective by 
this patch.


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] 4049: res_fax: Fix reference leak caused by gateway sessions being added to faxregistry.container twice

2014-10-14 Thread Corey Farrell

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

(Updated Oct. 14, 2014, 11:44 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 425457


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


Repository: Asterisk


Description
---

Reserved fax gateway sessions are added to faxregistry.container, then added 
again when the session is 'really' created.  It seems that when it is re-added 
it adds a second reference into the container due to the id being different.  
Removal of the original list entry is not successful.  This prevents the 
session from ever being unallocated.


Diffs
-

  /branches/1.8/res/res_fax.c 424175 

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


Testing
---

Verified this resolves the leak in 11 and 12.


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] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread Matt Jordan

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



branches/12/main/config.c
https://reviewboard.asterisk.org/r/4078/#comment24034

I'm not sure I'd dereference category after calling ast_category_destroy, 
even if its prev pointer is undisturbed. Since we've already assigned prev = 
category-prev, how about:

if (config-last_browse == category) {
config-last_browse = prev;
}



- Matt Jordan


On Oct. 14, 2014, 11:06 a.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4078/
 ---
 
 (Updated Oct. 14, 2014, 11:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
 while doing an ast_category_delete in an ast_category_browse loop.  
 Apparently this never worked but was also never tested.  I removed the test, 
 added 2 notes to config.h indicating that it's not supported and added a few 
 lines of code to ast_category_delete to prevent the SEGV should someone 
 attempt it in the future.  
 
 
 Diffs
 -
 
   branches/12/tests/test_config.c 425404 
   branches/12/main/config.c 425404 
   branches/12/include/asterisk/config.h 425404 
 
 Diff: https://reviewboard.asterisk.org/r/4078/diff/
 
 
 Testing
 ---
 
 The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
 tests still passes.
 
 
 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] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread George Joseph

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

(Updated Oct. 14, 2014, 11:13 a.m.)


Review request for Asterisk Developers.


Changes
---

Fixed a Duh pointed out by Matt.


Repository: Asterisk


Description
---

With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
while doing an ast_category_delete in an ast_category_browse loop.  Apparently 
this never worked but was also never tested.  I removed the test, added 2 notes 
to config.h indicating that it's not supported and added a few lines of code to 
ast_category_delete to prevent the SEGV should someone attempt it in the 
future.  


Diffs (updated)
-

  branches/12/tests/test_config.c 425404 
  branches/12/main/config.c 425404 
  branches/12/include/asterisk/config.h 425404 

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


Testing
---

The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
tests still passes.


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] 4076: res_phoneprov: Create accessor for ast_phoneprov_std_variable_lookup.

2014-10-14 Thread rmudgett

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


The undefined symbol warning is now gone on Asterisk startup.


branches/12/include/asterisk/phoneprov.h
https://reviewboard.asterisk.org/r/4076/#comment24035

Thge?



branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/4076/#comment24032

make static.

Should also make the following two arrays static as well. pp_user_lookup[] 
and pp_general_lookup[]



branches/12/res/res_phoneprov.c
https://reviewboard.asterisk.org/r/4076/#comment24033

Use AST_MODPRI_CHANNEL_DEPEND instead

This definitely needs to load after res_pjsip and before any other 
phoneprov providers such as res_pjsip_phoneprov_provider.


- rmudgett


On Oct. 13, 2014, 6:21 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4076/
 ---
 
 (Updated Oct. 13, 2014, 6:21 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Based on feedback from Richard, I created an accessor for 
 res_phoneprov/ast_phoneprov_std_variable_lookup and added load priority to 
 AST_MODULE_INFO.
 
 Richard, can you test this?  I don't see any difference in behavior before 
 and after the change.  res_phoneprov has AST_MODFLAG_GLOBAL_SYMBOLS set so it 
 should have always been loading before res_pjsip_phoneprov_provider which 
 doesn't have that flag set, regardless of load priority.  You can't even 
 modify that behavior in modules.conf.
 
 
 Diffs
 -
 
   branches/12/res/res_pjsip_phoneprov_provider.c 425404 
   branches/12/res/res_phoneprov.c 425404 
   branches/12/include/asterisk/phoneprov.h 425404 
 
 Diff: https://reviewboard.asterisk.org/r/4076/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] 4076: res_phoneprov: Create accessor for ast_phoneprov_std_variable_lookup.

2014-10-14 Thread George Joseph

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

(Updated Oct. 14, 2014, 12:02 p.m.)


Review request for Asterisk Developers and rmudgett.


Changes
---

Addressed Richard's comments.


Repository: Asterisk


Description
---

Based on feedback from Richard, I created an accessor for 
res_phoneprov/ast_phoneprov_std_variable_lookup and added load priority to 
AST_MODULE_INFO.

Richard, can you test this?  I don't see any difference in behavior before and 
after the change.  res_phoneprov has AST_MODFLAG_GLOBAL_SYMBOLS set so it 
should have always been loading before res_pjsip_phoneprov_provider which 
doesn't have that flag set, regardless of load priority.  You can't even modify 
that behavior in modules.conf.


Diffs (updated)
-

  branches/12/res/res_pjsip_phoneprov_provider.c 425404 
  branches/12/res/res_phoneprov.c 425404 
  branches/12/include/asterisk/phoneprov.h 425404 

Diff: https://reviewboard.asterisk.org/r/4076/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] 4076: res_phoneprov: Create accessor for ast_phoneprov_std_variable_lookup.

2014-10-14 Thread rmudgett

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

Ship it!


Ship It!

- rmudgett


On Oct. 14, 2014, 1:02 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4076/
 ---
 
 (Updated Oct. 14, 2014, 1:02 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Based on feedback from Richard, I created an accessor for 
 res_phoneprov/ast_phoneprov_std_variable_lookup and added load priority to 
 AST_MODULE_INFO.
 
 Richard, can you test this?  I don't see any difference in behavior before 
 and after the change.  res_phoneprov has AST_MODFLAG_GLOBAL_SYMBOLS set so it 
 should have always been loading before res_pjsip_phoneprov_provider which 
 doesn't have that flag set, regardless of load priority.  You can't even 
 modify that behavior in modules.conf.
 
 
 Diffs
 -
 
   branches/12/res/res_pjsip_phoneprov_provider.c 425404 
   branches/12/res/res_phoneprov.c 425404 
   branches/12/include/asterisk/phoneprov.h 425404 
 
 Diff: https://reviewboard.asterisk.org/r/4076/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] 4053: res_pjsip_history: A debugging module for busy systems

2014-10-14 Thread opticron

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



/trunk/res/res_pjsip_history.c
https://reviewboard.asterisk.org/r/4053/#comment24036

Shouldn't  be = instead?


- opticron


On Oct. 8, 2014, 8:55 a.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4053/
 ---
 
 (Updated Oct. 8, 2014, 8:55 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 While debugging things at SIPit, I found that the capabilities afforded by 
 res_pjsip_logger to be inadequate for tracing SIP messages on the CLI. Often, 
 so many requests/responses were received -- often with very large SDPs -- in 
 a short period of time that after a single call scenario, the initial 
 requests/responses had already been expunged from the CLI buffer. 
 Furthermore, displaying every single SIP request/response was often 
 counterproductive - OPTIONS, SUBSCRIBE, and even REGISTER requests were often 
 interleaved in the call scenarios, making it difficult to find or trace 
 portions of a call.
 
 This isn't the fault of res_pjsip_logger: it was doing exactly what it was 
 designed to do. And res_pjsip_logger is absolutely necessary for getting full 
 logs when a problem occurs. However, it isn't designed for debugging things 
 on the CLI. Hence, this module.
 
 res_pjsip_history records every request/response received/transmitted through 
 the PJSIP stack, but does not bother dumping them to the CLI. Instead, it 
 provides a few CLI comamnds to access the recorded messages:
 
 * pjsip set history {on|off} - enable/disable the history
 * pjsip show history [min [max]] - display the entire history, or segments of 
 the history.
 * pjsip show entry {num} - display a particular history entry
 
 Because we store all received/transmitted requests/responses, the module is 
 only suitable for debugging purposes. Leaving it permanently odd is a bad 
 idea, for obvious reasons. When the history is turned off, all messages are 
 purged and the history reset.
 
 As an example, we can record some history and display all of the messages:
 
 *CLI pjsip show history
 No.  Timestamp  Rx/Tx AddressCall-ID  SIP Message
  ==   
  1412775534.791 RX: 127.0.0.1:22428  d56324c8f042034aae29 OPTIONS 
 sip:127.0.0.1 SIP/2.0
 0001 1412775534.792 TX: 127.0.0.1:22428  d56324c8f042034aae29 SIP/2.0 200 OK
 0002 1412775540.277 RX: 127.0.0.1:22428  86cd74e458e76b79e267 OPTIONS 
 sip:127.0.0.1 SIP/2.0
 0003 1412775540.277 TX: 127.0.0.1:22428  86cd74e458e76b79e267 SIP/2.0 200 OK
 0004 1412775541.763 RX: 127.0.0.1:22428  f4c0050f5b604fc52ecc INVITE 
 sip:200@127.0.0.1 SIP/2.0
 0005 1412775541.765 TX: 127.0.0.1:22428  f4c0050f5b604fc52ecc SIP/2.0 200 OK
 0006 1412775541.766 TX: 127.0.0.1:22428  f4c0050f5b604fc52ecc SIP/2.0 200 OK
 0007 1412775541.780 RX: 127.0.0.1:22428  f4c0050f5b604fc52ecc ACK 
 sip:127.0.0.1:5060 SIP/2.0
 0008 1412775543.767 RX: 127.0.0.1:22428  f4c0050f5b604fc52ecc BYE 
 sip:127.0.0.1:5060 SIP/2.0
 0009 1412775543.768 TX: 127.0.0.1:22428  f4c0050f5b604fc52ecc SIP/2.0 200 OK
 0010 1412775547.823 RX: 127.0.0.1:22428  ab6fc9f37aa1dc5ed038 SUBSCRIBE 
 sip:1000@127.0.0.1 SIP/2.0
 0011 1412775547.824 TX: 127.0.0.1:22428  ab6fc9f37aa1dc5ed038 SIP/2.0 481 
 Call/Transaction Does Not Exist
 0012 1412775547.826 RX: 127.0.0.1:22428  ba4ed4625cbb3282b34c REGISTER 
 sip:127.0.0.1 SIP/2.0
 0013 1412775547.841 TX: 127.0.0.1:22428  ba4ed4625cbb3282b34c SIP/2.0 200 OK
 0014 1412775549.854 RX: 127.0.0.1:34899  ca022bf6e5a31b306bfd REGISTER 
 sip:127.0.0.1 SIP/2.0
 0015 1412775549.870 TX: 127.0.0.1:34899  ca022bf6e5a31b306bfd SIP/2.0 200 OK
 0016 1412775549.876 RX: 127.0.0.1:34899  d92c91d540a36d2cbca1 SUBSCRIBE 
 sip:1000@127.0.0.1 SIP/2.0
 0017 1412775549.876 TX: 127.0.0.1:34899  d92c91d540a36d2cbca1 SIP/2.0 200 OK
 0018 1412775549.877 TX: 127.0.0.1:34899  d92c91d540a36d2cbca1 NOTIFY 
 sip:1000@127.0.0.1:34899;transport=udp;registering_acc=127_0_0_1 SIP/2.0
 0019 1412775549.877 RX: 127.0.0.1:34899  32cca4f61cbba6cbe47f OPTIONS 
 sip:127.0.0.1 SIP/2.0
 0020 1412775549.877 TX: 127.0.0.1:34899  32cca4f61cbba6cbe47f SIP/2.0 200 OK
 0021 1412775549.889 RX: 127.0.0.1:34899  d92c91d540a36d2cbca1 SIP/2.0 200 OK
 0022 141277.376 RX: 127.0.0.1:34899  f65c7ea06475be757d2c INVITE 
 sip:1000@127.0.0.1 SIP/2.0
 0023 141277.377 TX: 127.0.0.1:34899  f65c7ea06475be757d2c SIP/2.0 487 
 Request Terminated
 0024 141277.382 TX: 127.0.0.1:34899  6ddb9f94-9c20-4f83-8 INVITE 
 sip:1000@127.0.0.1:34899;transport=udp;registering_acc=127_0_0_1 SIP/2.0
 0025 141277.383 TX: 

Re: [asterisk-dev] [Code Review] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-14 Thread Matt Jordan


 On Oct. 13, 2014, 2:04 p.m., rmudgett wrote:
  /branches/12/bridges/bridge_native_rtp.c, line 143
  https://reviewboard.asterisk.org/r/3997/diff/3/?file=68004#file68004line143
 
  Needs to be
  if (bc0 == bc1)
  to handle the first channel joining the bridge tech case.
 

If that's the case, then wouldn't bc1 be NULL?


- Matt


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


On Oct. 13, 2014, 12:32 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3997/
 ---
 
 (Updated Oct. 13, 2014, 12:32 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24327
 https://issues.asterisk.org/jira/browse/ASTERISK-24327
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a native RTP bridge that is remotely bridging its participants switches 
 to a softmix bridge, it may not properly re-INVITE the media for one or both 
 participants back to Asterisk. This is due to two factors:
 
 (1) The current bridge_native_rtp code only re-INVITEs if it believes the 
 channel will survive the bridge operation. Currently, that code is failing, 
 as it expects the channels to have a soft hangup flag set on it indicating 
 that a redirect has occurred or that the channel is going to leave the 
 bridge. (The code did not take into account a smart bridge operation).
 (2) When the bridge layer performs a smart bridge, it passes a dummy bridge 
 down into the old mixing technology when it is stopped. That breaks the 
 native RTP bridge, as it looks to bridge-channels to know which channels to 
 re-INVITE back. That list has no entries, as the dummy bridge does not 
 populate that value.
 
 This patch modifies bridge_native_rtp such that it keeps track of the 
 channels itself. Given how tricky this code is - both smart bridging and 
 native RTP bridging - this keeps the mixing technology insulated from changes 
 in the core, which is probably a good thing.
 
 
 Diffs
 -
 
   /branches/12/bridges/bridge_native_rtp.c 425404 
 
 Diff: https://reviewboard.asterisk.org/r/3997/diff/
 
 
 Testing
 ---
 
 The tests that extercised this code the most are the PJSIP blind transfer 
 tests, as they change the bridge mixing technology from native_rtp to simple 
 and back in various tests. Shocking the callee_with_hold/caller_with_hold 
 tests worked right off the bat. The direct media tests still fail, but this 
 is not surprising as the messages from Asterisk arrive interleaved, which is 
 not something SIPp handles well (at all).
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-14 Thread Joshua Colp

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



/branches/12/bridges/bridge_native_rtp.c
https://reviewboard.asterisk.org/r/3997/#comment24038

No, AST_LIST_FIRST and AST_LIST_LAST would return the same thing since it's 
a linked list containing one channel.


- Joshua Colp


On Oct. 13, 2014, 5:32 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3997/
 ---
 
 (Updated Oct. 13, 2014, 5:32 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24327
 https://issues.asterisk.org/jira/browse/ASTERISK-24327
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a native RTP bridge that is remotely bridging its participants switches 
 to a softmix bridge, it may not properly re-INVITE the media for one or both 
 participants back to Asterisk. This is due to two factors:
 
 (1) The current bridge_native_rtp code only re-INVITEs if it believes the 
 channel will survive the bridge operation. Currently, that code is failing, 
 as it expects the channels to have a soft hangup flag set on it indicating 
 that a redirect has occurred or that the channel is going to leave the 
 bridge. (The code did not take into account a smart bridge operation).
 (2) When the bridge layer performs a smart bridge, it passes a dummy bridge 
 down into the old mixing technology when it is stopped. That breaks the 
 native RTP bridge, as it looks to bridge-channels to know which channels to 
 re-INVITE back. That list has no entries, as the dummy bridge does not 
 populate that value.
 
 This patch modifies bridge_native_rtp such that it keeps track of the 
 channels itself. Given how tricky this code is - both smart bridging and 
 native RTP bridging - this keeps the mixing technology insulated from changes 
 in the core, which is probably a good thing.
 
 
 Diffs
 -
 
   /branches/12/bridges/bridge_native_rtp.c 425404 
 
 Diff: https://reviewboard.asterisk.org/r/3997/diff/
 
 
 Testing
 ---
 
 The tests that extercised this code the most are the PJSIP blind transfer 
 tests, as they change the bridge mixing technology from native_rtp to simple 
 and back in various tests. Shocking the callee_with_hold/caller_with_hold 
 tests worked right off the bat. The direct media tests still fail, but this 
 is not surprising as the messages from Asterisk arrive interleaved, which is 
 not something SIPp handles well (at all).
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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] 4071: scheduler: Fix a bug introduced by adding a delete flag to scheduled tasks

2014-10-14 Thread Jonathan Rose

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

(Updated Oct. 14, 2014, 1:49 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers, Matt Jordan and Mark Michelson.


Changes
---

Committed in revision 425503


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


Repository: Asterisk


Description
---

This issue was discovered by a rather complicated series of tests by PQ and 
it's somewhat intermittent relying on hitting the same race conditions that 
were being solved by r422070. When this problem hits the __sip_ack method in 
chan_sip in particular, things go south quickly.

The gist of it is that when we attempt to remove an existing task, we mark it 
for deletion and it is later removed from the scheduler. The deleted entry 
doesn't get free'd on account of the scheduler caching task structures so that 
we don't waste a bunch of effort reallocating all of the task structures every 
time a task needs to be created/torn down. When we wanted to remove a task that 
was currently executing, we couldn't do this immediately so we would apply a 
deleted flag. Unfortunately we didn't bother to clear the deleted flag when 
pulling it back off of the cache to create a new task.  Oops.  In any event, 
shenanigans ensued because the new task would be created already doomed and 
while they would be reported as successfully scheduled, ast_sched_runq would 
immediately delete the new task without replacing it the chan_sip __sip_ack 
function was anticipating that the tasks would stick around until it deleted 
them.

The fix is mind-numbingly simple for how long it took to me to figure out what 
the heck was going on... just remember to clear the deleted flag from scheduler 
entries when pulling them off the cache.


Diffs
-

  /branches/12/main/sched.c 425241 

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


Testing
---

We had a series of tests that, pre-patch would yield an assertion looking like 
the following:

[Oct 10 13:12:57] ERROR[18046][C-000e]: chan_sip.c:4428 __sip_ack: FRACK!, 
Failed assertion s != NULL, id=1570 (0)
Got 14 backtrace records
#0: [0x823d4fa] asterisk(__ast_assert_failed+0x7b) [0x823d4fa]
#1: [0x81fe913] asterisk(_ast_sched_del+0x2b3) [0x81fe913]
#2: [0xfeb248] /usr/lib/asterisk/modules/chan_sip.so [0xfeb248]
#3: [0x106c930] /usr/lib/asterisk/modules/chan_sip.so [0x106c930]
#4: [0x106d1e9] /usr/lib/asterisk/modules/chan_sip.so [0x106d1e9]
#5: [0x106cdb4] /usr/lib/asterisk/modules/chan_sip.so [0x106cdb4]
#6: [0x8171359] asterisk(ast_io_wait+0x14d) [0x8171359]
#7: [0x106f0f8] /usr/lib/asterisk/modules/chan_sip.so [0x106f0f8]
#8: [0x82399d5] asterisk [0x82399d5]d an infinite loop in the do_monitor thread 
couple with this set of log messages:

indicating that we were anticipating to find a scheduler entry that wasn't in 
the scheduler

Similar assertions occurred from other modules that involved schedulers but no 
chan_sip, but those didn't clearly break the world.

Typically a walk through the specified tests (which involved lots of chan_sip 
calls entering queues and lots of reloading of chan_sip and app_queue) would 
cause this breakdown to occur within one or two walks across the test series 
suggested. After performing the full set of tests 5 times each across Asterisk 
on two separate occasions without seeing any assertions of this type and 
without having chan_sip break down, I retested without the patch and quickly 
ran into the problem again. I think it's safe to say that I got it.


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] 4063: res_pjsip_session/res_pjsip_sdp_rtp: Fix a variety of situations where Asterisk would incorrectly reject offers

2014-10-14 Thread Matt Jordan

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

Review request for Asterisk Developers and Joshua Colp.


Bugs: ASTERISK-24122 and ASTERISK-24381
https://issues.asterisk.org/jira/browse/ASTERISK-24122
https://issues.asterisk.org/jira/browse/ASTERISK-24381


Repository: Asterisk


Description
---

When an inbound SDP offer is received, Asterisk currently makes a few 
incorrection assumptions:

(1) If the offer contains more than a single audio/video stream, Asterisk will 
reject the entire stream with a 488. This is an overly strict response; 
generally, Asterisk should accept the media streams that it can accept and 
decline the others.
(2) If the offer contains a declined media stream, Asterisk will attempt to 
process it anyway. This can result in attempting to match format capabilities 
on a declined media stream, leading to a 488. Asterisk should simply ignore 
declined media streams.
(3) Asterisk will currently attempt to handle offers with AVPF with 
use_avpf=No/AVP with use_avpf=Yes. This mismatch results in invalid SDP answers 
being sent in response. If there is a mismatch between the media type being 
offered and the configuration, Asterisk must reject the offer with a 488.

This patch does the following:
* Asterisk will accept SDP offers with at least one media stream that it can 
use. Some WARNING messages have been dropped to NOTICEs as a result.
* Asterisk will not accept an offer with a media type that doesn't match its 
configuration.
* Asterisk will ignore declined media streams properly.


Diffs
-

  /branches/13/res/res_pjsip_session.c 425502 
  /branches/13/res/res_pjsip_sdp_rtp.c 425502 

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


Testing
---

A large number of automated tests have been added for this change, along with 
basic SDP Offer/Answer semantics. See https://reviewboard.asterisk.org/r/4079


Thanks,

Matt Jordan

-- 
_
-- 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] 4079: testsuite: Update Offer/Answer PJSIP Tests

2014-10-14 Thread Matt Jordan

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

Review request for Asterisk Developers and Joshua Colp.


Repository: testsuite


Description
---

This patch heavily updates the existing PJSIP SDP Offer/Answer tests. Note that 
these tests only cover incoming SDP offer handling, and are not comprehensive 
to every scenario that could exist. However, they form a good baseline, and 
cover the changes in review 4063.

As a precursor, the existing tests have all been subsumed into the 
channels/pjsip/sdp_offer_answer/incoming/nominal/single-media-stream/audio 
tests. Because the purpose of these tests is to simply verify offer/answer 
semantics, other variants - such as varying transports - have been dropped. 
These variations are better tested in the basic call tests.

The following tests have been added:

- nominal
 - single-media-stream
  - audio
   - basic - test a variety of codecs under a single audio stream, matched to 
various PJSIP endpoint configurations
   - avpf - test media types (RTP/AVP|RTP/AVPF) in conjunction with the 
use_avpf setting
   - packetization - test ptime attribute handling
  - video
   - basic - similar to the audio/basic test, test a variety of codecs under a 
single video stream, matched to various PJSIP endpoint configurations
 - multiple-media-stream
  - audio - test multiple audio streams, where those streams may be initially 
declined
  - audio-video
   - basic - test a variety of codecs under a single audio/single video stream, 
matched to various PJSIP endpoint configurations
   - multiple-audio - test handling multiple audio streams with a single video 
stream
   - multiple-video - test handling multiple video streams with a single audio 
stream
  - audio-video-app - test handling streams that Asterisk has no hope of 
handling
  - video - test multiple video streams, where those streams may be initially 
declined
- off-nominal
 - multiple-media-stream
  - audio-video
   - codec-mismatch - test that media streams with no matching codec are 
declined, and that the entire Offer is 488'd if no media stream is acceptable
   - initial-declined - test that an offer with all declined media streams is 
488'd
  - audio
   - avpf-mismatch - test mismatched offers (RTP/AVP|RTP/AVPF) to endpoint 
configuration with the use_avpf setting
   - codec-mismatch - test inbound offers that contain no matching codecs in a 
single audio stream
   - initial-declined - test that an offer with a single declined audio stream 
is 488'd


Diffs
-

  /asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/tests.yaml 
5730 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/sipp/uac-declined.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/sipp/uac-declined-delayed.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/sipp/uac-codec-mismatch.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/avpf-mismatch/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/avpf-mismatch/sipp/uac-avpf-fail.xml
 PRE-CREATION 
  

Re: [asterisk-dev] [Code Review] 4079: testsuite: Update Offer/Answer PJSIP Tests

2014-10-14 Thread Matt Jordan

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

(Updated Oct. 14, 2014, 2:07 p.m.)


Review request for Asterisk Developers and Joshua Colp.


Changes
---

Add branch


Repository: testsuite


Description
---

This patch heavily updates the existing PJSIP SDP Offer/Answer tests. Note that 
these tests only cover incoming SDP offer handling, and are not comprehensive 
to every scenario that could exist. However, they form a good baseline, and 
cover the changes in review 4063.

As a precursor, the existing tests have all been subsumed into the 
channels/pjsip/sdp_offer_answer/incoming/nominal/single-media-stream/audio 
tests. Because the purpose of these tests is to simply verify offer/answer 
semantics, other variants - such as varying transports - have been dropped. 
These variations are better tested in the basic call tests.

The following tests have been added:

- nominal
 - single-media-stream
  - audio
   - basic - test a variety of codecs under a single audio stream, matched to 
various PJSIP endpoint configurations
   - avpf - test media types (RTP/AVP|RTP/AVPF) in conjunction with the 
use_avpf setting
   - packetization - test ptime attribute handling
  - video
   - basic - similar to the audio/basic test, test a variety of codecs under a 
single video stream, matched to various PJSIP endpoint configurations
 - multiple-media-stream
  - audio - test multiple audio streams, where those streams may be initially 
declined
  - audio-video
   - basic - test a variety of codecs under a single audio/single video stream, 
matched to various PJSIP endpoint configurations
   - multiple-audio - test handling multiple audio streams with a single video 
stream
   - multiple-video - test handling multiple video streams with a single audio 
stream
  - audio-video-app - test handling streams that Asterisk has no hope of 
handling
  - video - test multiple video streams, where those streams may be initially 
declined
- off-nominal
 - multiple-media-stream
  - audio-video
   - codec-mismatch - test that media streams with no matching codec are 
declined, and that the entire Offer is 488'd if no media stream is acceptable
   - initial-declined - test that an offer with all declined media streams is 
488'd
  - audio
   - avpf-mismatch - test mismatched offers (RTP/AVP|RTP/AVPF) to endpoint 
configuration with the use_avpf setting
   - codec-mismatch - test inbound offers that contain no matching codecs in a 
single audio stream
   - initial-declined - test that an offer with a single declined audio stream 
is 488'd


Diffs
-

  /asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/tests.yaml 
5730 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/sipp/uac-declined.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/sipp/uac-declined-delayed.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/sipp/uac-codec-mismatch.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/avpf-mismatch/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/avpf-mismatch/sipp/uac-avpf-fail.xml
 PRE-CREATION 
  

Re: [asterisk-dev] [Code Review] 4079: testsuite: Update Offer/Answer PJSIP Tests

2014-10-14 Thread Matt Jordan

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

(Updated Oct. 14, 2014, 2:07 p.m.)


Review request for Asterisk Developers and Joshua Colp.


Changes
---

Clean up some red blobs


Repository: testsuite


Description
---

This patch heavily updates the existing PJSIP SDP Offer/Answer tests. Note that 
these tests only cover incoming SDP offer handling, and are not comprehensive 
to every scenario that could exist. However, they form a good baseline, and 
cover the changes in review 4063.

As a precursor, the existing tests have all been subsumed into the 
channels/pjsip/sdp_offer_answer/incoming/nominal/single-media-stream/audio 
tests. Because the purpose of these tests is to simply verify offer/answer 
semantics, other variants - such as varying transports - have been dropped. 
These variations are better tested in the basic call tests.

The following tests have been added:

- nominal
 - single-media-stream
  - audio
   - basic - test a variety of codecs under a single audio stream, matched to 
various PJSIP endpoint configurations
   - avpf - test media types (RTP/AVP|RTP/AVPF) in conjunction with the 
use_avpf setting
   - packetization - test ptime attribute handling
  - video
   - basic - similar to the audio/basic test, test a variety of codecs under a 
single video stream, matched to various PJSIP endpoint configurations
 - multiple-media-stream
  - audio - test multiple audio streams, where those streams may be initially 
declined
  - audio-video
   - basic - test a variety of codecs under a single audio/single video stream, 
matched to various PJSIP endpoint configurations
   - multiple-audio - test handling multiple audio streams with a single video 
stream
   - multiple-video - test handling multiple video streams with a single audio 
stream
  - audio-video-app - test handling streams that Asterisk has no hope of 
handling
  - video - test multiple video streams, where those streams may be initially 
declined
- off-nominal
 - multiple-media-stream
  - audio-video
   - codec-mismatch - test that media streams with no matching codec are 
declined, and that the entire Offer is 488'd if no media stream is acceptable
   - initial-declined - test that an offer with all declined media streams is 
488'd
  - audio
   - avpf-mismatch - test mismatched offers (RTP/AVP|RTP/AVPF) to endpoint 
configuration with the use_avpf setting
   - codec-mismatch - test inbound offers that contain no matching codecs in a 
single audio stream
   - initial-declined - test that an offer with a single declined audio stream 
is 488'd


Diffs (updated)
-

  /asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/tests.yaml 
5730 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/tests.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/sipp/uac-declined.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/sipp/uac-declined-delayed.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/initial-declined/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/sipp/uac-codec-mismatch.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/codec-mismatch/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/avpf-mismatch/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/sdp_offer_answer/incoming/off-nominal/single-media-stream/audio/avpf-mismatch/sipp/uac-avpf-fail.xml
 PRE-CREATION 
  

Re: [asterisk-dev] [Code Review] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-14 Thread Matt Jordan


 On Oct. 14, 2014, 1:46 p.m., Joshua Colp wrote:
  /branches/12/bridges/bridge_native_rtp.c, line 143
  https://reviewboard.asterisk.org/r/3997/diff/3/?file=68004#file68004line143
 
  No, AST_LIST_FIRST and AST_LIST_LAST would return the same thing since 
  it's a linked list containing one channel.

Doh!


- Matt


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


On Oct. 13, 2014, 12:32 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3997/
 ---
 
 (Updated Oct. 13, 2014, 12:32 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24327
 https://issues.asterisk.org/jira/browse/ASTERISK-24327
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a native RTP bridge that is remotely bridging its participants switches 
 to a softmix bridge, it may not properly re-INVITE the media for one or both 
 participants back to Asterisk. This is due to two factors:
 
 (1) The current bridge_native_rtp code only re-INVITEs if it believes the 
 channel will survive the bridge operation. Currently, that code is failing, 
 as it expects the channels to have a soft hangup flag set on it indicating 
 that a redirect has occurred or that the channel is going to leave the 
 bridge. (The code did not take into account a smart bridge operation).
 (2) When the bridge layer performs a smart bridge, it passes a dummy bridge 
 down into the old mixing technology when it is stopped. That breaks the 
 native RTP bridge, as it looks to bridge-channels to know which channels to 
 re-INVITE back. That list has no entries, as the dummy bridge does not 
 populate that value.
 
 This patch modifies bridge_native_rtp such that it keeps track of the 
 channels itself. Given how tricky this code is - both smart bridging and 
 native RTP bridging - this keeps the mixing technology insulated from changes 
 in the core, which is probably a good thing.
 
 
 Diffs
 -
 
   /branches/12/bridges/bridge_native_rtp.c 425404 
 
 Diff: https://reviewboard.asterisk.org/r/3997/diff/
 
 
 Testing
 ---
 
 The tests that extercised this code the most are the PJSIP blind transfer 
 tests, as they change the bridge mixing technology from native_rtp to simple 
 and back in various tests. Shocking the callee_with_hold/caller_with_hold 
 tests worked right off the bat. The direct media tests still fail, but this 
 is not surprising as the messages from Asterisk arrive interleaved, which is 
 not something SIPp handles well (at all).
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-14 Thread Matt Jordan


 On Oct. 13, 2014, 2:04 p.m., rmudgett wrote:
  /branches/12/bridges/bridge_native_rtp.c, line 234
  https://reviewboard.asterisk.org/r/3997/diff/3/?file=68004#file68004line234
 
  Does glue1 need to be checked for NULL before use here?

No. You're guaranteed to have both bridge channels, and both channels have to 
be RTP capable for them to be in the bridge in the start/stop routines.


- Matt


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


On Oct. 13, 2014, 12:32 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3997/
 ---
 
 (Updated Oct. 13, 2014, 12:32 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24327
 https://issues.asterisk.org/jira/browse/ASTERISK-24327
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When a native RTP bridge that is remotely bridging its participants switches 
 to a softmix bridge, it may not properly re-INVITE the media for one or both 
 participants back to Asterisk. This is due to two factors:
 
 (1) The current bridge_native_rtp code only re-INVITEs if it believes the 
 channel will survive the bridge operation. Currently, that code is failing, 
 as it expects the channels to have a soft hangup flag set on it indicating 
 that a redirect has occurred or that the channel is going to leave the 
 bridge. (The code did not take into account a smart bridge operation).
 (2) When the bridge layer performs a smart bridge, it passes a dummy bridge 
 down into the old mixing technology when it is stopped. That breaks the 
 native RTP bridge, as it looks to bridge-channels to know which channels to 
 re-INVITE back. That list has no entries, as the dummy bridge does not 
 populate that value.
 
 This patch modifies bridge_native_rtp such that it keeps track of the 
 channels itself. Given how tricky this code is - both smart bridging and 
 native RTP bridging - this keeps the mixing technology insulated from changes 
 in the core, which is probably a good thing.
 
 
 Diffs
 -
 
   /branches/12/bridges/bridge_native_rtp.c 425404 
 
 Diff: https://reviewboard.asterisk.org/r/3997/diff/
 
 
 Testing
 ---
 
 The tests that extercised this code the most are the PJSIP blind transfer 
 tests, as they change the bridge mixing technology from native_rtp to simple 
 and back in various tests. Shocking the callee_with_hold/caller_with_hold 
 tests worked right off the bat. The direct media tests still fail, but this 
 is not surprising as the messages from Asterisk arrive interleaved, which is 
 not something SIPp handles well (at all).
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-14 Thread Matt Jordan

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

(Updated Oct. 14, 2014, 2:11 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Richard/Josh's findings


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


Repository: Asterisk


Description
---

When a native RTP bridge that is remotely bridging its participants switches to 
a softmix bridge, it may not properly re-INVITE the media for one or both 
participants back to Asterisk. This is due to two factors:

(1) The current bridge_native_rtp code only re-INVITEs if it believes the 
channel will survive the bridge operation. Currently, that code is failing, as 
it expects the channels to have a soft hangup flag set on it indicating that a 
redirect has occurred or that the channel is going to leave the bridge. (The 
code did not take into account a smart bridge operation).
(2) When the bridge layer performs a smart bridge, it passes a dummy bridge 
down into the old mixing technology when it is stopped. That breaks the native 
RTP bridge, as it looks to bridge-channels to know which channels to re-INVITE 
back. That list has no entries, as the dummy bridge does not populate that 
value.

This patch modifies bridge_native_rtp such that it keeps track of the channels 
itself. Given how tricky this code is - both smart bridging and native RTP 
bridging - this keeps the mixing technology insulated from changes in the core, 
which is probably a good thing.


Diffs (updated)
-

  /branches/12/bridges/bridge_native_rtp.c 425504 

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


Testing
---

The tests that extercised this code the most are the PJSIP blind transfer 
tests, as they change the bridge mixing technology from native_rtp to simple 
and back in various tests. Shocking the callee_with_hold/caller_with_hold tests 
worked right off the bat. The direct media tests still fail, but this is not 
surprising as the messages from Asterisk arrive interleaved, which is not 
something SIPp handles well (at all).


Thanks,

Matt Jordan

-- 
_
-- 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] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread wdoekes

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



branches/12/include/asterisk/config.h
https://reviewboard.asterisk.org/r/4078/#comment24041

Typo.


The rest is probably good, but I'll leave the shipit to someone else.

- wdoekes


On Oct. 14, 2014, 5:13 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4078/
 ---
 
 (Updated Oct. 14, 2014, 5:13 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
 while doing an ast_category_delete in an ast_category_browse loop.  
 Apparently this never worked but was also never tested.  I removed the test, 
 added 2 notes to config.h indicating that it's not supported and added a few 
 lines of code to ast_category_delete to prevent the SEGV should someone 
 attempt it in the future.  
 
 
 Diffs
 -
 
   branches/12/tests/test_config.c 425404 
   branches/12/main/config.c 425404 
   branches/12/include/asterisk/config.h 425404 
 
 Diff: https://reviewboard.asterisk.org/r/4078/diff/
 
 
 Testing
 ---
 
 The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
 tests still passes.
 
 
 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] 3948: Asterisk does not respect outbound proxy when sending qualify requests

2014-10-14 Thread Damian Ivereigh


 On Aug. 29, 2014, 11:04 p.m., Damian Ivereigh wrote:
  Thanks for all that info Matt. In answer to the question how should 
  outboundproxy behave, perhaps it might be useful to detail my setup. I 
  have a number of Asterisk servers on an internal network with a kamailio 
  server and a media proxy facing the internet. My goal was to harden the 
  kamailio server and allow the asterisk servers to be less secure and allow 
  each asterisk to define it's own peers. I want as close as possible for 
  each Asterisk server to appear to the outside world as if they are 
  externally connected (no NAT stuff), yet actually put everything through 
  kamailio and the media proxy.
  
  So the obvious solution was to use outboundproxy to get asterisk to send 
  its outgoing invites and registrations through the kamailio server which 
  would mangle them so that everything appeared to come from the external 
  server. However things fell apart when asterisk tried to send qualify 
  requests direct (which the firewall blocked). Hence this fix. I really 
  cannot see a situation where one would use an outboundproxy and then want 
  to send the qualify requests directly.
 
 wdoekes wrote:
 Re: mem leak: https://reviewboard.asterisk.org/r/4016/
 
 As for the how should the outboundproxy behave. I agree that this
 addition makes sense. But I'd like to hear someone else who uses
 obproxy to chime in too before giving this the go-ahead.
 
 Matt Jordan wrote:
 So, I haven't been able to drum up any feedback from those who use 
 outboundproxy.
 
 In the absence of any feedback, the only suggestion I have is that we 
 apply the patch and hope for the best. I'd ask that Damian be willing to help 
 respond to any outbound proxy issues that may arise as a result of this patch 
 (which I would say has minimal risk, given the confidence that the current 
 behavior is broken).


Yes I am willing to help!


- Damian


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


On Aug. 25, 2014, 10:04 p.m., Damian Ivereigh wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3948/
 ---
 
 (Updated Aug. 25, 2014, 10:04 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24063
 https://issues.asterisk.org/jira/browse/ASTERISK-24063
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The outboundproxy setting is ignored when sending the qualify packets 
 (OPTIONS). This means that if an asterisk server is unable to send the packet 
 directly to a peer, it is unable to qualify any non inbound registered peer 
 (e.g. a peer SIP Trunk). This problem is found on asterisk-11.6-cert4 (and 
 many others)
 
 It has been pointed out (thanks Walter Doekes), that the p-outboundproxy may 
 not be freed at the end which would create a memory leak.   
 
 
 Diffs
 -
 
   certified/tags/11.6-cert4/channels/chan_sip.c 422052 
 
 Diff: https://reviewboard.asterisk.org/r/3948/diff/
 
 
 Testing
 ---
 
 Have run this change in production for many months, however the possible 
 memory leak issue needs to be verified.
 
 
 Thanks,
 
 Damian Ivereigh
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread George Joseph

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

(Updated Oct. 14, 2014, 2:33 p.m.)


Review request for Asterisk Developers.


Changes
---

Fixed typo in config.h.


Repository: Asterisk


Description
---

With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
while doing an ast_category_delete in an ast_category_browse loop.  Apparently 
this never worked but was also never tested.  I removed the test, added 2 notes 
to config.h indicating that it's not supported and added a few lines of code to 
ast_category_delete to prevent the SEGV should someone attempt it in the 
future.  


Diffs (updated)
-

  branches/12/tests/test_config.c 425524 
  branches/12/main/config.c 425524 
  branches/12/include/asterisk/config.h 425524 

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


Testing
---

The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
tests still passes.


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] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread Matt Jordan

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

Ship it!


Ship It!

- Matt Jordan


On Oct. 14, 2014, 3:33 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4078/
 ---
 
 (Updated Oct. 14, 2014, 3:33 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
 while doing an ast_category_delete in an ast_category_browse loop.  
 Apparently this never worked but was also never tested.  I removed the test, 
 added 2 notes to config.h indicating that it's not supported and added a few 
 lines of code to ast_category_delete to prevent the SEGV should someone 
 attempt it in the future.  
 
 
 Diffs
 -
 
   branches/12/tests/test_config.c 425524 
   branches/12/main/config.c 425524 
   branches/12/include/asterisk/config.h 425524 
 
 Diff: https://reviewboard.asterisk.org/r/4078/diff/
 
 
 Testing
 ---
 
 The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
 tests still passes.
 
 
 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] 4078: config: Fix SEGV in unit test with MALLOC_DEBUG

2014-10-14 Thread George Joseph

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

(Updated Oct. 14, 2014, 3:46 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 425525


Repository: Asterisk


Description
---

With MALLOC_DEBUG the /main/config config_basic_ops test was causing a SEGV 
while doing an ast_category_delete in an ast_category_browse loop.  Apparently 
this never worked but was also never tested.  I removed the test, added 2 notes 
to config.h indicating that it's not supported and added a few lines of code to 
ast_category_delete to prevent the SEGV should someone attempt it in the 
future.  


Diffs
-

  branches/12/tests/test_config.c 425524 
  branches/12/main/config.c 425524 
  branches/12/include/asterisk/config.h 425524 

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


Testing
---

The unit tests pass now with MALLOC_DEBUG and the manager/config testsuite 
tests still passes.


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] 4075: parking/tests: Running res_parking unit tests would cause assertions and possibly a crash due to attempting to play MOH on a channel with no formats

2014-10-14 Thread Jonathan Rose

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

(Updated Oct. 14, 2014, 4:10 p.m.)


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Address a finding.

I also noticed that adding formats to this test caused the test channels to 
immediately be kicked out of the parking bridges causing the park_retrieval 
test to fail. To fix that I gave the channels read/write functions and a 
makeshift technology to use.


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


Repository: Asterisk


Description
---

This patch simply follows the suggested fix of specifying the format for test 
channels in the same manner as was done for the CDR unit tests.


Diffs (updated)
-

  /branches/13/res/parking/parking_tests.c 425404 

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


Testing
---

Ran tests prior to patch and got assertions.  Assertions no longer occurred 
with the patch in place.


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] 4082: Incomplete channel originate handling with ARI

2014-10-14 Thread greenfieldtech

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

This patch changes the current behavior of ARI, to allow channel originate 
requests to be performed with labels as the priority, not only integer values.


Diffs
-


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


Testing
---

Testing was performed by testing the following scenarios:
1. Originating a call to a numeric priority - works
2. Originating a call to a null priority - works
3. Originating a call to a label - works


File Attachments


ari_channels_label_2.diff
  
https://reviewboard.asterisk.org/media/uploaded/files/2014/10/14/b14484d7-c0df-4f89-9ab5-613d2f4f0f30__ari_channels_label_2.diff


Thanks,

greenfieldtech

-- 
_
-- 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] 4075: parking/tests: Running res_parking unit tests would cause assertions and possibly a crash due to attempting to play MOH on a channel with no formats

2014-10-14 Thread Jonathan Rose

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

(Updated Oct. 14, 2014, 4:49 p.m.)


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Improve the read function by making it just return the ast_null_frame pointer.


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


Repository: Asterisk


Description
---

This patch simply follows the suggested fix of specifying the format for test 
channels in the same manner as was done for the CDR unit tests.


Diffs (updated)
-

  /branches/13/res/parking/parking_tests.c 425404 

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


Testing
---

Ran tests prior to patch and got assertions.  Assertions no longer occurred 
with the patch in place.


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] 4083: testsuite: use replace instead of lstrip to remove portion of string

2014-10-14 Thread Scott Griepentrog

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

Review request for Asterisk Developers.


Repository: testsuite


Description
---

When a core dump occurs, if the test path started with t, e, or s, those 
characters would get stripped out from the path created to store the backtrace 
file.  This is due to incorrect usage of lstrip, which removes all instances of 
any of the provided characters from the start of the string regardless of the 
order:

 print 'tests/something'.lstrip('tests/')
omething

This patch changes lstrip to replace in several places, where the intent was 
clearly to remove only an exact string from another.


Diffs
-

  /asterisk/trunk/runtests.py 5730 
  /asterisk/trunk/lib/python/asterisk/sippversion.py 5730 

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


Testing
---


Thanks,

Scott Griepentrog

-- 
_
-- 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] 4075: parking/tests: Running res_parking unit tests would cause assertions and possibly a crash due to attempting to play MOH on a channel with no formats

2014-10-14 Thread rmudgett

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

Ship it!


Ship It!

- rmudgett


On Oct. 14, 2014, 4:49 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4075/
 ---
 
 (Updated Oct. 14, 2014, 4:49 p.m.)
 
 
 Review request for Asterisk Developers and Matt Jordan.
 
 
 Bugs: ASTERISK-24413
 https://issues.asterisk.org/jira/browse/ASTERISK-24413
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch simply follows the suggested fix of specifying the format for test 
 channels in the same manner as was done for the CDR unit tests.
 
 
 Diffs
 -
 
   /branches/13/res/parking/parking_tests.c 425404 
 
 Diff: https://reviewboard.asterisk.org/r/4075/diff/
 
 
 Testing
 ---
 
 Ran tests prior to patch and got assertions.  Assertions no longer occurred 
 with the patch in place.
 
 
 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