Re: [asterisk-dev] [Code Review] 4342: CHANNEL(peer), chan_iax2, res_fax, SNMP agent: Fix deadlock from reaching across a bridge.

2015-01-15 Thread rmudgett
Diff: https://reviewboard.asterisk.org/r/4342/diff/ Testing --- Testsuite tests still pass for FAX. The full testsuite didn't hit any deadlock. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api

Re: [asterisk-dev] [Code Review] 4336: app_dial: Don't publish DialEnd events twice if GOSUB_RESULT or MACRO_RESULT return an unexpected value

2015-01-15 Thread rmudgett
On Jan. 14, 2015, 1:06 p.m., rmudgett wrote: /branches/13/apps/app_dial.c, line 2977 https://reviewboard.asterisk.org/r/4336/diff/2/?file=70536#file70536line2977 Probably should set res9 and res on the gosub result conditions above then check res9 here since res may not be zero

Re: [asterisk-dev] [Code Review] 4340: FAX: Remove redundant locking.

2015-01-15 Thread rmudgett
- /branches/13/res/res_fax_spandsp.c 430609 /branches/13/res/res_fax.c 430609 Diff: https://reviewboard.asterisk.org/r/4340/diff/ Testing --- The testsuite fax tests still pass. Thanks, rmudgett -- _ -- Bandwidth

Re: [asterisk-dev] [Code Review] 4337: testsuite: Verify that bad MACRO_RESULT/GOSUB_RESULT values don't create multiple DialEnd events

2015-01-15 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4337/#review14201 --- Ship it! That's all I was looking at. :) - rmudgett On Jan

Re: [asterisk-dev] [Code Review] 4349: Asterisk: Creating a named ARI bridge twice causes a crash

2015-01-15 Thread rmudgett
/resource_bridges.c https://reviewboard.asterisk.org/r/4349/#comment24662 ast_null_safe_strcmp() is not really needed here. You have just checked to see if the args-name string is not empty and the bridge-name can never be NULL since it is a stringfield. - rmudgett On Jan. 15, 2015, 4:44 p.m., Ashley

Re: [asterisk-dev] [Code Review] 4336: app_dial: Don't publish DialEnd events twice if GOSUB_RESULT or MACRO_RESULT return an unexpected value

2015-01-14 Thread rmudgett
/r/4336/#comment24648 Probably should set res9 and res on the gosub result conditions above then check res9 here since res may not be zero on entering the enclosing code block. - rmudgett On Jan. 14, 2015, 12:07 p.m., Matt Jordan wrote

Re: [asterisk-dev] [Code Review] 4338: res_pjsip_outbound_registration: Fix race condition with executing pjsip show registrations and reloading.

2015-01-14 Thread rmudgett
of exiting early is now when there is no state to go with the previous configuration, the container could be non-empty but not print anything. The ast_sip_cli_traverse_objects() would then not notify the user about No objects found. - rmudgett On Jan. 14, 2015, 1:28 p.m., Joshua Colp wrote

Re: [asterisk-dev] [Code Review] 4340: FAX: Remove redundant locking.

2015-01-14 Thread rmudgett
/branches/13/res/res_fax.c 430609 Diff: https://reviewboard.asterisk.org/r/4340/diff/ Testing --- The testsuite fax tests still pass. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api

Re: [asterisk-dev] [Code Review] 4337: testsuite: Verify that bad MACRO_RESULT/GOSUB_RESULT values don't create multiple DialEnd events

2015-01-14 Thread rmudgett
/test-config.yaml https://reviewboard.asterisk.org/r/4337/#comment24647 This should be app_stack not app_macro - rmudgett On Jan. 14, 2015, 12:08 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit

[asterisk-dev] [Code Review] 4340: FAX: Remove redundant locking.

2015-01-14 Thread rmudgett
--- The testsuite fax tests still 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

[asterisk-dev] [Code Review] 4342: CHANNEL(peer), chan_iax, res_fax, SNMP agent: Fix deadlock from reaching across a bridge.

2015-01-14 Thread rmudgett
430625 /branches/13/res/res_fax.c 430625 /branches/13/funcs/func_channel.c 430625 /branches/13/channels/chan_iax2.c 430625 Diff: https://reviewboard.asterisk.org/r/4342/diff/ Testing --- Testsuite tests still pass for FAX. The full testsuite didn't hit any deadlock. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4342: CHANNEL(peer), chan_iax2, res_fax, SNMP agent: Fix deadlock from reaching across a bridge.

2015-01-14 Thread rmudgett
/func_channel.c 430625 /branches/13/channels/chan_iax2.c 430625 Diff: https://reviewboard.asterisk.org/r/4342/diff/ Testing --- Testsuite tests still pass for FAX. The full testsuite didn't hit any deadlock. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4337: testsuite: Verify that bad MACRO_RESULT/GOSUB_RESULT values don't create multiple DialEnd events

2015-01-14 Thread rmudgett
://reviewboard.asterisk.org/r/4337/#comment24642 Need to add missing dependencies: asterisk: 'app_dial' asterisk: 'app_echo' asterisk: 'app_macro' asterisk: 'app_userevent' - rmudgett On Jan. 13, 2015, 9 p.m., Matt Jordan wrote

Re: [asterisk-dev] [Code Review] 4336: app_dial: Don't publish DialEnd events twice if GOSUB_RESULT or MACRO_RESULT return an unexpected value

2015-01-14 Thread rmudgett
? /branches/13/apps/app_dial.c https://reviewboard.asterisk.org/r/4336/#comment24644 Should this be just if (res) instead? - rmudgett On Jan. 13, 2015, 9 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail

Re: [asterisk-dev] [Code Review] 4292: app_macro: Don't restore the calling location on a channel redirect.

2015-01-13 Thread rmudgett
://reviewboard.asterisk.org/r/4319/ Tests park_timeout_inside and redirect_inside fail when patch is not applied. All four tests pass when patch is applied. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api

Re: [asterisk-dev] [Code Review] 4319: testsuite: app_macro tests for channel redirect while the macro is active.

2015-01-13 Thread rmudgett
/4292/ is applied. Tests 2 and 4 fail when the patch is not applied. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit

Re: [asterisk-dev] [Code Review] 4319: testsuite: app_macro tests for channel redirect while the macro is active.

2015-01-12 Thread rmudgett
in that macro context, which header is available is version dependent and the Extension header might eventually dissapear in favor of the Exten header. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-09 Thread rmudgett
/app_agent_pool.c 430433 /branches/13/UPGRADE.txt 430433 /branches/13/CHANGES 430433 Diff: https://reviewboard.asterisk.org/r/4315/diff/ Testing --- Issued all of the AMI actions listed above to verify that the output was consistent. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-09 Thread rmudgett
() in ami_subscription_detail(). Diffs (updated) - /branches/13/res/res_pjsip_outbound_publish.c 430354 Diff: https://reviewboard.asterisk.org/r/4315/diff/ Testing --- Issued all of the AMI actions listed above to verify that the output was consistent. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-09 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4315/#review14147 --- This is not the correct diff for this review. - rmudgett

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-09 Thread rmudgett
/apps/app_agent_pool.c 430433 /branches/13/UPGRADE.txt 430433 /branches/13/CHANGES 430433 Diff: https://reviewboard.asterisk.org/r/4315/diff/ Testing --- Issued all of the AMI actions listed above to verify that the output was consistent. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-09 Thread rmudgett
On Jan. 9, 2015, 10:31 a.m., rmudgett wrote: This is not the correct diff for this review. Diff 3 that is. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4315

Re: [asterisk-dev] [Code Review] 4319: testsuite: app_macro tests for channel redirect while the macro is active.

2015-01-09 Thread rmudgett
the patch is not applied. 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

Re: [asterisk-dev] [Code Review] 4320: res_fax: Make T.38 negotiation timeout configurable and handle T.38 switch failure

2015-01-08 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4320/#review14122 --- Ship it! Ship It! - rmudgett On Jan. 7, 2015, 7:25 p.m

Re: [asterisk-dev] [Code Review] 4322: app_bridge: return to next dialplan priority

2015-01-08 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4322/#review14124 --- Ship it! Ship It! - rmudgett On Jan. 7, 2015, 5:10 p.m

Re: [asterisk-dev] [Code Review] 4320: res_fax: Make T.38 negotiation timeout configurable and handle T.38 switch failure

2015-01-07 Thread rmudgett
/4320/#comment24619 Indentation could be reduced. - rmudgett On Jan. 7, 2015, 5:29 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4320

Re: [asterisk-dev] [Code Review] 4320: res_fax: Make T.38 negotiation timeout configurable and handle T.38 switch failure

2015-01-07 Thread rmudgett
between variable declarations and code to make it easier to find the declarations. - rmudgett On Jan. 7, 2015, 3:20 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r

[asterisk-dev] [Code Review] 4319: testsuite: app_macro tests for channel redirect while the macro is active.

2015-01-07 Thread rmudgett
-CREATION Diff: https://reviewboard.asterisk.org/r/4319/diff/ Testing --- All tests pass when the patch on review https://reviewboard.asterisk.org/r/4292/ is applied. Tests 2 and 4 fail when the patch is not applied. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4292: app_macro: Don't restore the calling location on a channel redirect.

2015-01-07 Thread rmudgett
that covers this patch. Tests up on review at https://reviewboard.asterisk.org/r/4319/ - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4292/#review14027

Re: [asterisk-dev] [Code Review] 4292: app_macro: Don't restore the calling location on a channel redirect.

2015-01-07 Thread rmudgett
is not applied. All four tests pass when patch is applied. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-07 Thread rmudgett
additional AMI headers for the event. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4315/#review14086 --- On Jan. 2, 2015, 6

Re: [asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-07 Thread rmudgett
/app_agent_pool.c 430354 /branches/13/UPGRADE.txt 430354 /branches/13/CHANGES 430354 Diff: https://reviewboard.asterisk.org/r/4315/diff/ Testing --- Issued all of the AMI actions listed above to verify that the output was consistent. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4292: app_macro: Don't restore the calling location on a channel redirect.

2015-01-05 Thread rmudgett
. On the v13 version of the patch, the parked call returns to the configured macro extension with the patch. Without the patch, the call was immediately reparked. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http

[asterisk-dev] [Code Review] 4315: AMI: Make AMI actions that generate event lists consistent.

2015-01-02 Thread rmudgett
to verify that the output was consistent. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

Re: [asterisk-dev] [Code Review] 4281: Testsuite: Single channel redirect tests

2014-12-23 Thread rmudgett
as dependencies - rmudgett On Dec. 19, 2014, 3:02 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4281

Re: [asterisk-dev] [Code Review] 4282: queue_log: Post QUEUESTART entry when Asterisk fully boots.

2014-12-22 Thread rmudgett
/asterisk.c 429698 /branches/11/include/asterisk/_private.h 429698 Diff: https://reviewboard.asterisk.org/r/4282/diff/ Testing --- The QUEUESTART entry is now written to the queue_log file when Asterisk boots rather than waiting for the first event to happen. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4259: DTMF atxfer: Setup recall channels as if the original transferrer originated the call.

2014-12-22 Thread rmudgett
a callid for the recall calls where before the recall calls didn't have any callid. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update

[asterisk-dev] [Code Review] 4292: app_macro: Don't restore the calling location on a channel redirect.

2014-12-22 Thread rmudgett
returns to the configured macro extension with the patch. Without the patch, the call was immediately reparked. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing

Re: [asterisk-dev] [Code Review] 4292: app_macro: Don't restore the calling location on a channel redirect.

2014-12-22 Thread rmudgett
, the call was immediately reparked. 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

Re: [asterisk-dev] [Code Review] 4271: ARI: Allow interoperation of ASYNCGOTO and channels originated to Stasis()

2014-12-17 Thread rmudgett
/r/4271/#comment24507 You should check if the channel has a PBX first. If the channel has a PBX you need to keep the ASYNCGOTO flag so the PBX that called the Stasis() application can goto the new dialplan location. - rmudgett On Dec. 17, 2014, 11:54 a.m., opticron wrote

Re: [asterisk-dev] [Code Review] 4271: ARI: Allow interoperation of ASYNCGOTO and channels originated to Stasis()

2014-12-17 Thread rmudgett
ASYNCGOTO chan_hungup = ast_check_hangup() unlock_channel if !chan_hungup run PBX on channel. - rmudgett On Dec. 17, 2014, 12:30 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply

Re: [asterisk-dev] [Code Review] 4271: ARI: Allow interoperation of ASYNCGOTO and channels originated to Stasis()

2014-12-17 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4271/#review13994 --- Ship it! OK by me. - rmudgett On Dec. 17, 2014, 1:52 p.m

Re: [asterisk-dev] [Code Review] 4259: DTMF atxfer: Setup recall channels as if the original transferrer originated the call.

2014-12-15 Thread rmudgett
where before the recall calls didn't have any callid. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

Re: [asterisk-dev] [Code Review] 4259: DTMF atxfer: Setup recall channels as if the original transferrer originated the call.

2014-12-12 Thread rmudgett
as just COLP with the private party id removed. I'll have to think about what common code to pull into a routine. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4259/#review13953

Re: [asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-12 Thread rmudgett
to run the TestSuite with DEBUG_THREADS enabled on the http://svn.asterisk.org/svn/asterisk/team/rmudgett/debug_threads branch nightly for a few weeks. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api

Re: [asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-12 Thread rmudgett
issues on Asterisk startup (ASTERISK-19463) have been a hard problem to reproduce, I propose we setup Bamboo to run the TestSuite with DEBUG_THREADS enabled on the http://svn.asterisk.org/svn/asterisk/team/rmudgett/debug_threads branch nightly for a few weeks. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4262: bridge: channel ref leak on blond_nonfinal_enter

2014-12-12 Thread rmudgett
://reviewboard.asterisk.org/r/4262/#comment24483 Without looking further, the channel refs here seem to be mismatched. - rmudgett On Dec. 12, 2014, 4:10 p.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https

[asterisk-dev] [Code Review] 4259: DTMF atxfer: Setup recall channels as if the original transferrer originated the call.

2014-12-11 Thread rmudgett
COLP information such as B's name. Also core show channel B shows channel variables that wouldn't be there without the patch. * The debug log now has a callid for the recall calls where before the recall calls didn't have any callid. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-10 Thread rmudgett
code. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4247/#review13936 --- On Dec. 9, 2014, 11:21 a.m., rmudgett wrote

Re: [asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-09 Thread rmudgett
mutex initialization code, you are right that it probably should be uncommented now. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4247/#review13929

Re: [asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-09 Thread rmudgett
with DEBUG_THREADS enabled on the http://svn.asterisk.org/svn/asterisk/team/rmudgett/debug_threads branch nightly for a few weeks. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com

Re: [asterisk-dev] [Code Review] 4108: Weak References

2014-12-09 Thread rmudgett
released. /trunk/tests/test_astobj2_weaken.c https://reviewboard.asterisk.org/r/4108/#comment24457 This should be fail_cleanup1 so obj gets cleaned up. - rmudgett On Oct. 26, 2014, 6:10 a.m., Corey Farrell wrote

Re: [asterisk-dev] [Code Review] 4246: PJSIP: Stagger outbound qualifies

2014-12-08 Thread rmudgett
://reviewboard.asterisk.org/r/4246/#comment24447 You need to check that the returned value is never zero. qualify_frequency could become zero if the user disables qualify on a config reload. - rmudgett On Dec. 8, 2014, 1:16 p.m., opticron wrote

Re: [asterisk-dev] [Code Review] 4246: PJSIP: Stagger outbound qualifies

2014-12-08 Thread rmudgett
://reviewboard.asterisk.org/r/4246/#comment24449 Ok. Then this comment can just be removed. - rmudgett On Dec. 8, 2014, 1:16 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https

[asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-08 Thread rmudgett
, it ran over the weekend without a problem. Since the DEBUG_THREADS locking issues on Asterisk startup (ASTERISK-19463) have been a hard problem to reproduce, I propose we setup Bamboo to run the TestSuite with DEBUG_THREADS enabled on the http://svn.asterisk.org/svn/asterisk/team/rmudgett

Re: [asterisk-dev] [Code Review] 4247: DEBUG_THREADS: Fix regression and lock tracking initialization problems.

2014-12-08 Thread rmudgett
problem to reproduce, I propose we setup Bamboo to run the TestSuite with DEBUG_THREADS enabled on the http://svn.asterisk.org/svn/asterisk/team/rmudgett/debug_threads branch nightly for a few weeks. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4242: app_meetme: Fix default values initialization when no configuration file is present

2014-12-05 Thread rmudgett
. The defaults need to be set in load_config_meetme() after calling ast_config_load() if a reload. trunk/apps/app_meetme.c https://reviewboard.asterisk.org/r/4242/#comment24414 Pass reload to load_config_meetme() so it can determine when it needs to set defaults. - rmudgett On Dec. 5

Re: [asterisk-dev] [Code Review] 4242: app_meetme: Fix default values initialization when no configuration file is present

2014-12-05 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4242/#review13905 --- Ship it! Ship It! - rmudgett On Dec. 5, 2014, 12:49 p.m

Re: [asterisk-dev] [Code Review] 4242: app_meetme: Fix default values initialization when no configuration file is present

2014-12-05 Thread rmudgett
On Dec. 5, 2014, 1:31 p.m., rmudgett wrote: Ship It! This should be applied to v11+ as this patch is really a bug fix. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r

Re: [asterisk-dev] [Code Review] 4243: ari: Add the ability to specify a linkedId when originating calls.

2014-12-05 Thread rmudgett
/resource_channels.c https://reviewboard.asterisk.org/r/4243/#comment24417 This channel cannot be made to /continue in dialplan when the Stasis application exits. - rmudgett On Dec. 5, 2014, 2:28 p.m., Joshua Colp wrote

Re: [asterisk-dev] [Code Review] 4243: ari: Add the ability to specify a linkedId when originating calls.

2014-12-05 Thread rmudgett
://reviewboard.asterisk.org/r/4243/#comment24419 Should only have one variable declaration per line. /branches/13/res/ari/resource_channels.c https://reviewboard.asterisk.org/r/4243/#comment24420 Heh. This doesn't match what the last sentence in the review description is saying. - rmudgett

Re: [asterisk-dev] [Code Review] 4231: New AMI/ARI events for connected line updates on a channel

2014-12-04 Thread rmudgett
://reviewboard.asterisk.org/r/4231/#comment24402 NewCallerid? How about NewConnectedLine instead. :) - rmudgett On Dec. 4, 2014, 11:07 a.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 4199: test_channel_feature_hooks.c: Fix unit test for DTMF hooks.

2014-11-24 Thread rmudgett
/test_channel_feature_hooks.c 428271 /branches/13/main/bridge_channel.c 428271 /branches/13/main/bridge.c 428271 Diff: https://reviewboard.asterisk.org/r/4199/diff/ Testing --- The unit test now passes. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4200: manager: Fix could not extend string messages.

2014-11-21 Thread rmudgett
, using a 128 constant instead of 256 would be appropriate. Just calculated the max string would be ~121 as there are comma separators to account for. 128 would still work but with very little margin for any new entries. I'll change it to 150. - rmudgett

Re: [asterisk-dev] [Code Review] 4200: manager: Fix could not extend string messages.

2014-11-21 Thread rmudgett
://reviewboard.asterisk.org/r/4200/diff/ Testing --- With the patch, the failed to extend from %d to %d messages no longer happen when shutting down Asterisk that has an AMI connection. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided

Re: [asterisk-dev] [Code Review] 4194: ast_str: Fix improper member access to struct ast_str members.

2014-11-19 Thread rmudgett
with the 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] 4196: bridge_basic: Fix features issues introduced by review 4167

2014-11-19 Thread rmudgett
://reviewboard.asterisk.org/r/4196/#comment24295 Would we want these sounds to be interruptible with a digit for the next attempt? Note an ast_stopstream(chan) is needed to actually stop a stream interrupted by a digit if so. - rmudgett On Nov. 19, 2014, 11:35 a.m., Matt Jordan wrote

Re: [asterisk-dev] [Code Review] 4196: bridge_basic: Fix features issues introduced by review 4167

2014-11-19 Thread rmudgett
messages and before the retry_sound/invalid_sound playback you could then easily have those sounds interrupted by a digit to put into exten. - rmudgett On Nov. 19, 2014, 4:57 p.m., Matt Jordan wrote: --- This is an automatically generated e

[asterisk-dev] [Code Review] 4199: test_channel_feature_hooks.c: Fix unit test for DTMF hooks.

2014-11-19 Thread rmudgett
://reviewboard.asterisk.org/r/4199/diff/ Testing --- The unit test now passes. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit

[asterisk-dev] [Code Review] 4200: manager: Fix could not extend string messages.

2014-11-19 Thread rmudgett
that has an AMI connection. 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

Re: [asterisk-dev] [Code Review] 4186: stringfields: Fix regression from fix for unintentional memory retention and another issue exposed by the fix

2014-11-18 Thread rmudgett
but with a reserved space of 8 characters starting at position 2 and ending as indicated by A - rmudgett On Nov. 18, 2014, 8:25 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 4186: stringfields: Fix regression from fix for unintentional memory retention caused by ast_string_fields_copy

2014-11-18 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4186/#review13806 --- Ship it! Ship It! - rmudgett On Nov. 18, 2014, 12:36 p.m

Re: [asterisk-dev] [Code Review] 4184: app_confbridge: Don't delay playing 'you have been kicked' prompts to end_marked users when there are only end_marked users in the conference

2014-11-14 Thread rmudgett
playing a prompt to an empty bridge is pointless. Another way might be to play the leader has left prompt and then kick the end_marked users out. However, this would be more of a behaviour change and may have consistency issues with leader participant counts. - rmudgett On Nov. 14, 2014, 2

Re: [asterisk-dev] [Code Review] 4123: app_agent_pool: Make agent alert interruptable by DTMF.

2014-11-06 Thread rmudgett
hooks starting with '*'. When I pressed the '*', the rewinding of the playback was delayed by the digit collection interdigit timeout time but the audio was not discarded during the interdigit timeout time. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4123: app_agent_pool: Make agent alert interruptable by DTMF.

2014-11-05 Thread rmudgett
sizeof would have worked just as well in this case. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4123/#review13676

Re: [asterisk-dev] [Code Review] 4123: app_agent_pool: Make agent alert interruptable by DTMF.

2014-11-05 Thread rmudgett
pressed the '*', the rewinding of the playback was delayed by the digit collection interdigit timeout time but the audio was not discarded during the interdigit timeout time. Thanks, rmudgett -- _ -- Bandwidth and Colocation

Re: [asterisk-dev] [Code Review] 4114: Prevent stringfields from accumulating unused memory

2014-11-03 Thread rmudgett
when target != *ptr. Probably should change to using __p__ instead of ptr to avoid having unexpected type problems dealing with ptr. - rmudgett On Nov. 3, 2014, 7:29 a.m., Corey Farrell wrote: --- This is an automatically

Re: [asterisk-dev] [Code Review] 4114: Prevent stringfields from accumulating unused memory

2014-11-03 Thread rmudgett
On Oct. 29, 2014, 6:55 p.m., rmudgett wrote: 1) In stringfields.h:ast_string_field_ptr_set_by_fields(), the __p__ and ptr pointers are the same by initialization so the test for *__p__ != *ptr is always false and will not release the old string value when

Re: [asterisk-dev] [Code Review] 4123: app_agent_pool: Make agent alert interruptable by DTMF.

2014-11-03 Thread rmudgett
the '*', the rewinding of the playback was delayed by the digit collection interdigit timeout time but the audio was not discarded during the interdigit timeout time. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http

Re: [asterisk-dev] [Code Review] 4137: res_pjsip: Add disable_tcp_switch option.

2014-11-03 Thread rmudgett
to TCP for this option to prevent. 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

Re: [asterisk-dev] [Code Review] 4114: Prevent stringfields from accumulating unused memory

2014-11-03 Thread rmudgett
memory leak? /branches/11/include/asterisk/stringfields.h https://reviewboard.asterisk.org/r/4114/#comment24171 Use __p__ instead of ptr to reduce potential confusion here because of the mixed use of __p__ and ptr. - rmudgett On Nov. 3, 2014, 11:55 a.m., Corey Farrell wrote

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

2014-10-31 Thread rmudgett
://reviewboard.asterisk.org/r/4128/#comment24164 Those ast_frfree's can be done now for completeness. - rmudgett On Oct. 30, 2014, 9:43 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 4135: Resolve race condition that can result in ARI apps not being notified of transfers

2014-10-31 Thread rmudgett
/4135/#comment24165 I have a feeling that the race also affects parking here. If so then holding the bridge lock is not a viable solution. - rmudgett On Oct. 31, 2014, 10:58 a.m., Mark Michelson wrote

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

2014-10-31 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4128/#review13652 --- Ship it! Ship It! - rmudgett On Oct. 31, 2014, 12:20 p.m

Re: [asterisk-dev] [Code Review] 4135: Resolve race condition that can result in ARI apps not being notified of transfers

2014-10-31 Thread rmudgett
On Oct. 31, 2014, 12:39 p.m., rmudgett wrote: /branches/12/main/bridge.c, line 4253 https://reviewboard.asterisk.org/r/4135/diff/2/?file=68675#file68675line4253 I have a feeling that the race also affects parking here. If so then holding the bridge lock is not a viable solution

Re: [asterisk-dev] [Code Review] 4135: Resolve race condition that can result in ARI apps not being notified of transfers

2014-10-30 Thread rmudgett
places a little later. Just don't forget to unlock before calling blind_transfer_bridge() below. :) /branches/12/main/bridge.c https://reviewboard.asterisk.org/r/4135/#comment24151 Bridge not locked before the goto here. - rmudgett On Oct. 30, 2014, 5:51 p.m., Mark Michelson

Re: [asterisk-dev] [Code Review] 4128: func_jitterbuffer: fix frame leaks

2014-10-30 Thread rmudgett
. /branches/11/funcs/func_jitterbuffer.c https://reviewboard.asterisk.org/r/4128/#comment24157 ast_frfree(frame) before replacing with null frame. - rmudgett On Oct. 30, 2014, 7:06 p.m., Corey Farrell wrote

[asterisk-dev] [Code Review] 4137: res_pjsip: Add disable_tcp_switch option.

2014-10-30 Thread rmudgett
--- Tested the documentation and setting of the new option. I'm not sure what system setup is needed to trigger pjproject to try switching from UDP to TCP for this option to prevent. Thanks, rmudgett -- _ -- Bandwidth

Re: [asterisk-dev] [Code Review] 4114: Prevent stringfields from accumulating unused memory

2014-10-29 Thread rmudgett
one string from one pool. Once the string is found in a pool you don't need to continue looking in any remaining pools. Setting pool-used = 0 is a good catch for the first pool as this fixes reclaiming the space of the first pool. - rmudgett On Oct. 27, 2014, 3:20 a.m., Corey Farrell

Re: [asterisk-dev] [Code Review] 4111: app_queue: ao2_iterator not destroyed, causing leak

2014-10-27 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4111/#review13598 --- Ship it! Ship It! - rmudgett On Oct. 27, 2014, 12:53 a.m

Re: [asterisk-dev] [Code Review] 4105: codec_dahdi: Fix segfault on load.

2014-10-22 Thread rmudgett
. branches/13/codecs/codec_dahdi.c https://reviewboard.asterisk.org/r/4105/#comment24106 Need to update the doxygen function parameter documentation. - rmudgett On Oct. 22, 2014, 2:13 p.m., Shaun Ruffell wrote

Re: [asterisk-dev] [Code Review] 4105: codec_dahdi: Fix segfault on load.

2014-10-22 Thread rmudgett
On Oct. 22, 2014, 2:33 p.m., rmudgett wrote: Minor nits. The commit description has using using. Also the commit description would need to have the following headers added: ASTERISK-24435 #close Reported by: Marian Koniuszko Review: https://reviewboard.asterisk.org/r/4105

Re: [asterisk-dev] [Code Review] 4100: codec_dahdi: Fix crash on load of codec_dahdi.

2014-10-22 Thread rmudgett
some calls that perform translation. No crashes happened. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

[asterisk-dev] [Code Review] 4100: codec_dahdi: Fix crash on load of codec_dahdi.

2014-10-21 Thread rmudgett
- /branches/13/main/translate.c 426078 /branches/13/include/asterisk/translate.h 426078 /branches/13/codecs/codec_dahdi.c 426078 Diff: https://reviewboard.asterisk.org/r/4100/diff/ Testing --- Made some calls that perform translation. No crashes happened. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4074: AMI: Add missing VarSet events when a channel inherits variables.

2014-10-16 Thread rmudgett
variable inheritance and the initial Local;2 channel variables obtained from Local;1. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update

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
://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

Re: [asterisk-dev] [Code Review] 4066: stasis_channels.c: Resolve unfinished Dials when doing masquerades (Part 2)

2014-10-14 Thread rmudgett
. 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

Re: [asterisk-dev] [Code Review] 4076: res_phoneprov: Create accessor for ast_phoneprov_std_variable_lookup.

2014-10-14 Thread rmudgett
. - 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

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

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

<    1   2   3   4   5   6   7   8   9   >