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

2014-08-26 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/#review13186 --- Ship it! Ship It! - rmudgett On Aug. 26, 2014, 5:43 p.m

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

2014-08-25 Thread rmudgett
off. If not, reset state. */ That isn't a good comment as it just restates the code. I'll add a comment giving the why. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3928

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

2014-08-25 Thread rmudgett
/res_musiconhold.c 421902 Diff: https://reviewboard.asterisk.org/r/3928/diff/ Testing --- Establish a call between two parties and place one party on and off hold several times. MOH resumes where it left off from the last hold. Thanks, rmudgett

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

2014-08-25 Thread rmudgett
. If anything, this should be done when the channel is moved out of stasis to a non-stasis controlled bridge as part of the move hook. - rmudgett On Aug. 22, 2014, 11:13 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail

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

2014-08-25 Thread rmudgett
static const struct quote_set escape_sets[] branches/1.8/tests/test_utils.c https://reviewboard.asterisk.org/r/3923/#comment23633 static const struct quote_set escape_sets[] - rmudgett On Aug. 25, 2014, 8:52 a.m., opticron wrote

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

2014-08-25 Thread rmudgett
. - rmudgett On Aug. 25, 2014, 3:50 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3923/ --- (Updated Aug

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

2014-08-25 Thread rmudgett
muted. branches/12/apps/app_confbridge.c https://reviewboard.asterisk.org/r/3944/#comment23640 This is set but not used since it is set to -2 by the next use. - rmudgett On Aug. 25, 2014, 4:43 p.m., George Joseph wrote

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

2014-08-22 Thread rmudgett
party on and off hold several times. MOH resumes where it left off from the last hold. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE

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

2014-08-22 Thread rmudgett
/3927/#comment23619 Initing cond must be done only once so it should be done in sched_alloc(). sched_alloc() either creates a new sched object or gets it from a cache. - rmudgett On Aug. 22, 2014, 2:39 p.m., Mark Michelson wrote

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

2014-08-21 Thread rmudgett
/#comment23608 Line should be wrapped before ao2_bump. - rmudgett On Aug. 21, 2014, 11:30 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3908

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

2014-08-21 Thread rmudgett
://reviewboard.asterisk.org/r/3908/#comment23616 Missed an unref recoring here. You really didn't need to eliminate the RAII_VAR here because there were multiple return points that would have a recording to cleanup. but oh well. - rmudgett On Aug. 21, 2014, 2:44 p.m., Jonathan Rose wrote

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

2014-08-21 Thread rmudgett
://reviewboard.asterisk.org/r/3908/#comment23618 Remove now inaccurate comment. - rmudgett On Aug. 21, 2014, 3:11 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https

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

2014-08-21 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3926/#review13155 --- Ship it! Ship It! - rmudgett On Aug. 21, 2014, 5:45 p.m

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

2014-08-20 Thread rmudgett
420978 /branches/13/main/bridge_channel.c 420978 /branches/13/main/bridge.c 420978 /branches/13/channels/chan_pjsip.c 420978 Diff: https://reviewboard.asterisk.org/r/3906/diff/ Testing --- Performed blind SIP transfers (REFER) and the call still passes media. Thanks, rmudgett

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

2014-08-20 Thread rmudgett
and the caller would not know if the data was destroyed. It might be better to have command_create() do this instead if the command object could not be created. This way the other places that create commands would be covered as well. - rmudgett On Aug. 20, 2014, 4:21 p.m., Jonathan Rose wrote

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

2014-08-19 Thread rmudgett
transfer gives correct party id information to all parties involved. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit

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

2014-08-19 Thread rmudgett
functions differ more than just locking the channel. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3920/#review13118

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

2014-08-19 Thread rmudgett
[] = { { name number, name, number }, { \name\ number, name, number }, { name, name, NULL }, { 1234, NULL, 1234 } } The last two entries above are also valid possibilities for the input string to ast_callerid_parse(). - rmudgett On Aug. 18, 2014, 8:55 p.m., opticron wrote

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

2014-08-19 Thread rmudgett
/manager.c https://reviewboard.asterisk.org/r/3915/#comment23578 Channel is not required. If not supplied then all channels are to be processed. - rmudgett On Aug. 19, 2014, 7:16 a.m., opticron wrote: --- This is an automatically

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

2014-08-19 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3921/#review13122 --- Ship it! Looks ok to me. - rmudgett On Aug. 19, 2014, 7:29

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

2014-08-18 Thread rmudgett
be either malloced memory or an ao2 object. - rmudgett On Aug. 14, 2014, 1:46 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3908

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

2014-08-18 Thread rmudgett
zealous being an ERROR. We recently got rid of a similar over zealous WARNING message in URIENCODE/URIDECODE (See ASTERISK-23911). branches/12/funcs/func_config.c https://reviewboard.asterisk.org/r/3919/#comment23554 Make this ast_debug(1, ... - rmudgett On Aug. 18, 2014, 11:46 a.m

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

2014-08-18 Thread rmudgett
://reviewboard.asterisk.org/r/3920/#comment23555 Could be simplified a bit: int hangup_flag; hangup_flag = ast_bridge_setup_after_goto(chan) ? AST_SOFTHANGUP_DEV : AST_SOFTHANGUP_ASYNCGOTO; ast_channel_lock(chan) ast_softhangup_nolock(chan, hangup_flag) ast_channel_unlock(chan) - rmudgett

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

2014-08-18 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3919/#review13107 --- Ship it! Ship It! - rmudgett On Aug. 18, 2014, 1:47 p.m

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

2014-08-18 Thread rmudgett
://reviewboard.asterisk.org/r/3921/#comment23556 Please make this one declaration per line. json_transferer is leaked (new and pre-existing leak) json_transferee is leaked (new) - rmudgett On Aug. 18, 2014, 1:55 p.m., opticron wrote

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

2014-08-18 Thread rmudgett
://reviewboard.asterisk.org/r/3915/#comment23560 ast_channel_connected_effective_id() is a somewhat more expensive operation than ast_channel_connected(). It would be best if the party_id were saved to a local copy rather than repeatedly called. - rmudgett On Aug. 15, 2014, 9:59 a.m., opticron wrote

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

2014-08-14 Thread rmudgett
is the same. Diffs - /branches/13/main/channel_internal_api.c 420838 /branches/13/main/channel.c 420838 Diff: https://reviewboard.asterisk.org/r/3904/diff/ Testing --- Compiler is still happy. Thanks, rmudgett

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

2014-08-14 Thread rmudgett
/resource_channels.c 420855 Diff: https://reviewboard.asterisk.org/r/3905/diff/ Testing --- ARI originate of a local channel still goes out. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api

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

2014-08-14 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3910/#review13086 --- Ship it! Ship It! - rmudgett On Aug. 14, 2014, 1:28 p.m

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

2014-08-14 Thread rmudgett
: https://reviewboard.asterisk.org/r/3913/diff/ Testing --- Attended transfer gives correct party id information to all parties involved. Blind transfer gives correct party id information to all parties involved. Thanks, rmudgett

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

2014-08-13 Thread rmudgett
: https://reviewboard.asterisk.org/r/3906/diff/ Testing --- Performed blind SIP transfers (REFER) and the call still passes media. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com

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

2014-08-11 Thread rmudgett
/r/3903/#comment23514 This variable is effectively unused. Nothing seems to care about the value set in res. That is unless you actually intend to return the value but forgot to. - rmudgett On Aug. 11, 2014, 1:35 p.m., opticron wrote

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

2014-08-11 Thread rmudgett
line continuation. /branches/12/main/channel_internal_api.c https://reviewboard.asterisk.org/r/3900/#comment23518 Why were these changed? They were already using _nolock suffix. ast_channel_set_unbridged() and ast_channel_set_unbridged_nolock() - rmudgett On Aug. 11, 2014, 1:23 p.m

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

2014-08-11 Thread rmudgett
/r/3900/#comment23519 Remove unbridged references. Also there are extra parens around the ASYNCGOTO flag. - rmudgett On Aug. 11, 2014, 1:23 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit

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

2014-08-11 Thread rmudgett
/r/3900/#comment23522 double parens ((...)) /branches/12/main/framehook.c https://reviewboard.asterisk.org/r/3900/#comment23523 _nolock version /branches/12/main/framehook.c https://reviewboard.asterisk.org/r/3900/#comment23524 _nolock version - rmudgett On Aug. 11, 2014, 4

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

2014-08-11 Thread rmudgett
://reviewboard.asterisk.org/r/3904/diff/ Testing --- Compiler is still happy. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit

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

2014-08-11 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3900/#review13081 --- Ship it! Ship It! - rmudgett On Aug. 11, 2014, 5:06 p.m

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

2014-08-11 Thread rmudgett
. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-08-09 Thread rmudgett
the issue for me. Please file an issue on the issue tracker as this patch has been commited. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/#review13058

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

2014-08-08 Thread rmudgett
be closed as discarded as the other review has been committed. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3829/#review12905

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

2014-08-08 Thread rmudgett
instead always clear the unbridge flag when a channel joins the bridging system. - rmudgett On Aug. 8, 2014, 5:42 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https

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

2014-08-07 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3723/#review13049 --- Ship it! Ship It! - rmudgett On Aug. 7, 2014, 9:23 a.m

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

2014-08-07 Thread rmudgett
for the configured codecs header. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman

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

2014-08-07 Thread rmudgett
been committed. - rmudgett On June 20, 2014, 9:06 a.m., Alexander Traud wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3653

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

2014-08-07 Thread rmudgett
feature. Appropriate changes need to be made to the sample config files as well as a CHANGES note. - rmudgett On July 30, 2014, 10:07 a.m., Michael K. wrote: --- This is an automatically generated e-mail. To reply, visit: https

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

2014-08-06 Thread rmudgett
the previous call to ast_tcptls_stream_set_exclusive_input() in _sip_tcp_helper_thread() tells it not to wait. Also please change the if statement as walter mentioned. Use of the bitwise or in that manner is unusual. Where did you see this done elsewhere? - rmudgett

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

2014-08-06 Thread rmudgett
only kind of change. The warning is benign and can be ignored as it is an alembic only warning. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

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

2014-08-06 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3882/#review13038 --- Ship it! Ship It! - rmudgett On Aug. 6, 2014, 1:04 p.m

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

2014-08-06 Thread rmudgett
On Aug. 6, 2014, 2:22 p.m., rmudgett wrote: Ship It! I'll commit this tomorrow if you don't have commit access. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3882

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

2014-08-06 Thread rmudgett
of ast_free_ptr. You don't need to pass a pointer since the RAII_VAR() macro creates a local function that actually calls ast_free(). In fact, using ast_free() allows MALLOC_DEBUG to know where the block was actually freed. - rmudgett On Aug. 6, 2014, 10:44 a.m., Mark Michelson wrote

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

2014-08-05 Thread rmudgett
be followed. Also as stated, IAX2 doesn't know how to negotiate any codec options. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3889/#review12982

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

2014-08-05 Thread rmudgett
--- Compiles and code inspeciton. See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert(). The format_list container is used to generate the format_list_array which is then is used by chan_iax2 to determine the format index sent over the wire in IAX_IE_CODEC_PREFS. Thanks, rmudgett

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

2014-08-05 Thread rmudgett
, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev

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

2014-08-05 Thread rmudgett
with iax2 pass. * The iax.conf bandwidth option is now functional and no longer causes a frack when a call is made. * The CLI iax2 show peer has enough buffer space to generate a longer codec list for the configured codecs header. Thanks, rmudgett

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

2014-08-04 Thread rmudgett
/3882/#comment23363 Should also test for EINTR and do the same thing as for EAGAIN. - rmudgett On July 31, 2014, 1:14 p.m., ebroad wrote: --- This is an automatically generated e-mail. To reply, visit: https

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

2014-08-04 Thread rmudgett
as accepting. See LargeBinary http://docs.sqlalchemy.org/en/rel_0_9/core/types.html I'll augment the comment. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3870/#review12974

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

2014-08-04 Thread rmudgett
be ignored as it is an alembic only warning. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com

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

2014-08-04 Thread rmudgett
() and format_pref.c:ast_codec_pref_convert(). The format_list container is used to generate the format_list_array which is then is used by chan_iax2 to determine the format index sent over the wire in IAX_IE_CODEC_PREFS. Thanks, rmudgett

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

2014-08-04 Thread rmudgett
://reviewboard.asterisk.org/r/3890/diff/ Testing --- * The iax.conf bandwidth option is now functional and no longer causes a frack when a call is made. * The CLI iax2 show peer has enough buffer space to generate a longer codec list for the configured codecs header. Thanks, rmudgett

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

2014-08-04 Thread rmudgett
://reviewboard.asterisk.org/r/3890/#comment23380 Fixed comment to read: Array is ordered by preference. Contains the iax2_supported_formats[] index + 1. - rmudgett On Aug. 4, 2014, 7:41 p.m., rmudgett wrote: --- This is an automatically

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

2014-07-31 Thread rmudgett
the issue, plus changing TLS would be a much more invasive operation. In my opinion, we should remove the use of FILE handles altogether in the TCP/TLS code [...] Yes. There still is fgets() in chan_sip.c, it should be killed. rmudgett wrote

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

2014-07-31 Thread rmudgett
the issue, plus changing TLS would be a much more invasive operation. In my opinion, we should remove the use of FILE handles altogether in the TCP/TLS code [...] Yes. There still is fgets() in chan_sip.c, it should be killed. rmudgett wrote

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

2014-07-31 Thread rmudgett
the issue, plus changing TLS would be a much more invasive operation. In my opinion, we should remove the use of FILE handles altogether in the TCP/TLS code [...] Yes. There still is fgets() in chan_sip.c, it should be killed. rmudgett wrote

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

2014-07-30 Thread rmudgett
://reviewboard.asterisk.org/r/3603/#comment23297 And function curly on its own line. - rmudgett On June 13, 2014, 1:48 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r

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

2014-07-30 Thread rmudgett
/3833/#comment23301 free() and ast_free() are NULL tolerant. However, it is likely better to set vmu-email = NULL after freeing like other places. - rmudgett On July 21, 2014, 4:54 p.m., Jason Parker wrote

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

2014-07-30 Thread rmudgett
and downgrade modes for mysql and postgres. The queue_members change does give a postgres warning message from the postgres alembic backend about a mysql only kind of change. The warning is benign and can be ignored as it is an alembic only warning. Thanks, rmudgett

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

2014-07-28 Thread rmudgett
/res/stasis/app.c https://reviewboard.asterisk.org/r/3865/#comment23250 Is OBJ_NOLOCK needed here? It wasn't used on the ao2_find above. - rmudgett On July 28, 2014, 9:56 a.m., opticron wrote: --- This is an automatically generated

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

2014-07-28 Thread rmudgett
mutex so OBJ_NOLOCK really should not be used at all. In the case of this patch have you obtained it? I don't think so. - rmudgett On July 28, 2014, 11:41 a.m., opticron wrote: --- This is an automatically generated e-mail

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

2014-07-28 Thread rmudgett
/ Testing --- Code inspection and compiler. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

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

2014-07-28 Thread rmudgett
for v11 over the v1.8 audit at https://reviewboard.asterisk.org/r/3859/ Diffs - /branches/11/funcs/func_jitterbuffer.c 419680 Diff: https://reviewboard.asterisk.org/r/3860/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett

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

2014-07-28 Thread rmudgett
/3860/ Diffs - /branches/12/main/channel.c 419680 /branches/12/main/abstract_jb.c 419680 Diff: https://reviewboard.asterisk.org/r/3861/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett

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

2014-07-25 Thread rmudgett
On July 24, 2014, 4:01 p.m., rmudgett wrote: You are affecting the lifetime of the continer node object without protecting it. It can now potentially be used outside the lifetime of its container as part of the held object. The container node and the continer ponter

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

2014-07-25 Thread rmudgett
On July 24, 2014, 4:01 p.m., rmudgett wrote: You are affecting the lifetime of the continer node object without protecting it. It can now potentially be used outside the lifetime of its container as part of the held object. The container node and the continer ponter

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

2014-07-25 Thread rmudgett
On July 24, 2014, 4:01 p.m., rmudgett wrote: You are affecting the lifetime of the continer node object without protecting it. It can now potentially be used outside the lifetime of its container as part of the held object. The container node and the continer ponter

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

2014-07-25 Thread rmudgett
and calling ao2_cleanup(). - rmudgett On July 25, 2014, 8:20 a.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3852

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

2014-07-25 Thread rmudgett
to eliminate a return path. - rmudgett On July 25, 2014, 4:30 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3852

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

2014-07-25 Thread rmudgett
/func_frame_trace.c 419627 /branches/1.8/apps/app_speech_utils.c 419627 /branches/1.8/apps/app_queue.c 419627 Diff: https://reviewboard.asterisk.org/r/3859/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett

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

2014-07-25 Thread rmudgett
/app_speech_utils.c 419627 /branches/1.8/apps/app_queue.c 419627 Diff: https://reviewboard.asterisk.org/r/3859/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http

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

2014-07-25 Thread rmudgett
--- Triggered DYNAMIC_FEATURES mapped to the following applications: Playback Gosub Macro The Playback played the file, the Gosub and Macro applications executed their respective dialplan sections. Thanks, rmudgett -- _ -- Bandwidth

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

2014-07-25 Thread rmudgett
: https://reviewboard.asterisk.org/r/3860/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

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

2014-07-25 Thread rmudgett
/3861/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

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

2014-07-25 Thread rmudgett
/funcs/func_jitterbuffer.c 419680 Diff: https://reviewboard.asterisk.org/r/3860/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com

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

2014-07-25 Thread rmudgett
- /branches/1.8/funcs/func_frame_trace.c 419627 /branches/1.8/apps/app_speech_utils.c 419627 /branches/1.8/apps/app_queue.c 419627 Diff: https://reviewboard.asterisk.org/r/3859/diff/ Testing --- Code inspection and compiler. Thanks, rmudgett

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

2014-07-24 Thread rmudgett
check. This can probably be done at the same time as waiting for the pending destructor collision nodes to go away. - rmudgett On July 7, 2014, 11:43 p.m., George Joseph wrote: --- This is an automatically generated e-mail

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

2014-07-24 Thread rmudgett
://reviewboard.asterisk.org/r/3840/#comment23234 Maybe add a: Kicking everything from bridge '%s'. message. - rmudgett On July 23, 2014, 4:53 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit

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

2014-07-24 Thread rmudgett
/3840/#comment23235 This was the RAII_VAR I was pointing out. - rmudgett On July 24, 2014, 4:51 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3840

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

2014-07-24 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3840/#review12867 --- Ship it! Ship It! - rmudgett On July 24, 2014, 4:56 p.m

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

2014-07-24 Thread rmudgett
change. Note the peeraccount value is meaningless if the bridge does not contain two parties. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

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

2014-07-24 Thread rmudgett
--- All accountcode tagged tests pass. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman

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

2014-07-23 Thread rmudgett
executed their respective dialplan sections. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2

2014-07-22 Thread rmudgett
://reviewboard.asterisk.org/r/3759/#comment23140 You should get a iterator ref for ast_sched_add() before calling it and unref if it fails to be added. Otherwise, you run the risk of iterator being stale when ast_sched_add succeeds. - rmudgett On July 21, 2014, 10:52 p.m., Corey Farrell wrote

Re: [asterisk-dev] [Code Review] 3813: codec_speex: Fix trashing normal static frame for AST_FRAME_CNG.

2014-07-22 Thread rmudgett
. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 3728: ARI: Add missing transfer information

2014-07-22 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3728/#review12808 --- Ship it! Ship It! - rmudgett On July 18, 2014, 12:42 p.m

Re: [asterisk-dev] [Code Review] 3731: Stasis: Prevent non-stasis channels from entering stasis bridges

2014-07-22 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3731/#review12809 --- team/rmudgett/stasis_linkedids/res/stasis/app.c https

Re: [asterisk-dev] [Code Review] 3729: Stasis: Allow prestart actions to be queued

2014-07-22 Thread rmudgett
value. The way you had it counting each command executed before will always give you the number of commands actually executed. - rmudgett On July 21, 2014, 4:55 p.m., opticron wrote: --- This is an automatically generated e-mail

Re: [asterisk-dev] [Code Review] 3731: Stasis: Prevent non-stasis channels from entering stasis bridges

2014-07-22 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3731/#review12813 --- Ship it! Ship It! - rmudgett On July 22, 2014, 1:08 p.m

Re: [asterisk-dev] [Code Review] 3836: app_bridgewait: Remove race condition where bridge may be dissolved when trying to join

2014-07-22 Thread rmudgett
new channels from entering the BridgeWait holding bridge until all channels that entered the bridging system in that holding bridge hangup. It could take awhile if a channel was moved to a normal bridge. - rmudgett On July 22, 2014, 10:14 a.m., Joshua Colp wrote

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

2014-07-22 Thread rmudgett
61 https://reviewboard.asterisk.org/r/3832/diff/1/?file=64932#file64932line61 I don't think you need the dependency on func_channel for this test. With so many tests it was getting difficult to keep those dependencies straight. :) - rmudgett

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2

2014-07-22 Thread rmudgett
/3759/#comment23168 Try -1 instead :) - rmudgett On July 22, 2014, 5:01 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3759

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

2014-07-21 Thread rmudgett
/test_case.py 5288 /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 5288 Diff: https://reviewboard.asterisk.org/r/3832/diff/ Testing --- All accountcode tagged tests pass. Thanks, rmudgett -- _ -- Bandwidth

<    1   2   3   4   5   6   7   8   9   >