Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE
On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
I don't see what happens with the phone-context argument. Shouldn't we pass
that on as a channel variable?
Geert Van Pamel wrote:
We return this into the hostport.
Geert Van Pamel wrote:
According to RFC 3966 phone-context is either a domain-name, or (part of)
an international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the
local number must be unique within its context...
Olle E Johansson wrote:
So it ends up in the SIPDOMAIN variable in the dial plan? It has to be
reachable in the dial plan somehow.
Geert Van Pamel wrote:
The variable ${SIPDOMAIN} contains the local IP address of the Asterisk
server.
The userinfo arrives in ${CALLERID} and is displayed on the display of
the called device, and arrives in the CDR file.
Actually I do not know into which variable the incoming hostport info is
copied to?
Could somebody else answer this question?
If I place a normal call to sip:ge...@example.com to my Asterisk server.
geert will be the extension I'm looking for, example.com ends up in
SIPDOMAIN. It's not the local IP address, it's the domain/host part of the
request URI in the INVITE.
I would prefer if phone context ended up in TELPHONECONTEXT so I could use it
the same way as SIPDOMAIN in the dial plan. It should not end up in SIPDOMAIN
as it is not a SIP uri. That way an extension in a local context can be routed
differently than an extension in a global context.
- Olle E
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/#review11323
---
On March 22, 2014, 2:08 p.m., Geert Van Pamel wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/
---
(Updated March 22, 2014, 2:08 p.m.)
Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt
Jordan, and wdoekes.
Bugs: ASTERISK-17179
https://issues.asterisk.org/jira/browse/ASTERISK-17179
Repository: Asterisk
Description
---
Implements RFC-3966 TEL URI incoming INVITE.
See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description
of the original isssue.
I have been patching all versions since Asterisk 1.6. I would like to include
the code into the main trunk for version 13.
Previously Asterisk was failing with error on incoming IMS call:
Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address
missing 'sip:', using it anyway
Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a
SIP header (tel:0987654321;phone-context=+32987654321)?
Reason: tel: protocol was not recognized.
Diffs
-
/trunk/channels/sip/reqresp_parser.c 410429
/trunk/channels/chan_sip.c 410429
Diff: https://reviewboard.asterisk.org/r/3349/diff/
Testing
---
Executed an incoming TEL URI INVITE connection.
CLI was present on the display and in the CDR file.
No errors on SIP debug output.
File Attachments
RFC-3966 tel URI patch
https://reviewboard.asterisk.org/media/uploaded/files/2014/03/13/cad7a996-88c1-47fe-a2a9-cc6987af3b75__rfc-3966-tel-uri-patch-diff.txt
Thanks,
Geert Van Pamel
--
_
-- 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] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3346/ --- (Updated March 24, 2014, 9:03 a.m.) Review request for Asterisk Developers. Changes --- Minor cleanups. Bugs: ASTERISK-23351 https://issues.asterisk.org/jira/browse/ASTERISK-23351 Repository: Asterisk Description --- This patch fixes handling of nullable int columns in update_realtime function. It checks if a value is empty and sets the column to NULL instead of '', which raises an error. Additionally, it checks for existence of the keyfield column instead of the first parameter column. Diffs (updated) - http://svn.asterisk.org/svn/asterisk/branches/1.8/res/res_config_pgsql.c 411017 Diff: https://reviewboard.asterisk.org/r/3346/diff/ Testing --- Only tested for successful compilation. Someone needs to confirm that the patch works fine. File Attachments Patch version for branch 12 https://reviewboard.asterisk.org/media/uploaded/files/2014/03/17/743ba069-05bf-4b31-9bc9-08af861a6d84__res_config_pgsql.12.diff Thanks, zvision -- _ -- 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] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime
On March 23, 2014, 3:46 p.m., wdoekes wrote: Minor nits. Still waiting for someone to test this. Sure, no hurry:) - zvision --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3346/#review11333 --- On March 24, 2014, 9:03 a.m., zvision wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3346/ --- (Updated March 24, 2014, 9:03 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23351 https://issues.asterisk.org/jira/browse/ASTERISK-23351 Repository: Asterisk Description --- This patch fixes handling of nullable int columns in update_realtime function. It checks if a value is empty and sets the column to NULL instead of '', which raises an error. Additionally, it checks for existence of the keyfield column instead of the first parameter column. Diffs - http://svn.asterisk.org/svn/asterisk/branches/1.8/res/res_config_pgsql.c 411017 Diff: https://reviewboard.asterisk.org/r/3346/diff/ Testing --- Only tested for successful compilation. Someone needs to confirm that the patch works fine. File Attachments Patch version for branch 12 https://reviewboard.asterisk.org/media/uploaded/files/2014/03/17/743ba069-05bf-4b31-9bc9-08af861a6d84__res_config_pgsql.12.diff Thanks, zvision -- _ -- 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] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3346/ --- (Updated March 24, 2014, 9:11 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23351 https://issues.asterisk.org/jira/browse/ASTERISK-23351 Repository: Asterisk Description --- This patch fixes handling of nullable int columns in update_realtime function. It checks if a value is empty and sets the column to NULL instead of '', which raises an error. Additionally, it checks for existence of the keyfield column instead of the first parameter column. Diffs - http://svn.asterisk.org/svn/asterisk/branches/1.8/res/res_config_pgsql.c 411017 Diff: https://reviewboard.asterisk.org/r/3346/diff/ Testing --- Only tested for successful compilation. Someone needs to confirm that the patch works fine. Thanks, zvision -- _ -- 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] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3346/ --- (Updated March 24, 2014, 9:11 a.m.) Review request for Asterisk Developers. Changes --- New patch version for branch 12 Bugs: ASTERISK-23351 https://issues.asterisk.org/jira/browse/ASTERISK-23351 Repository: Asterisk Description --- This patch fixes handling of nullable int columns in update_realtime function. It checks if a value is empty and sets the column to NULL instead of '', which raises an error. Additionally, it checks for existence of the keyfield column instead of the first parameter column. Diffs - http://svn.asterisk.org/svn/asterisk/branches/1.8/res/res_config_pgsql.c 411017 Diff: https://reviewboard.asterisk.org/r/3346/diff/ Testing --- Only tested for successful compilation. Someone needs to confirm that the patch works fine. File Attachments (updated) Patch version for branch 12 https://reviewboard.asterisk.org/media/uploaded/files/2014/03/24/de074116-1aff-4fbb-a2f3-50fa54e5cac9__res_config_pgsql.12.diff Thanks, zvision -- _ -- 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] 3349: Implement RFC-3966 TEL URI incoming INVITE
On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
I don't see what happens with the phone-context argument. Shouldn't we pass
that on as a channel variable?
Geert Van Pamel wrote:
We return this into the hostport.
Geert Van Pamel wrote:
According to RFC 3966 phone-context is either a domain-name, or (part of)
an international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the
local number must be unique within its context...
Olle E Johansson wrote:
So it ends up in the SIPDOMAIN variable in the dial plan? It has to be
reachable in the dial plan somehow.
Geert Van Pamel wrote:
The variable ${SIPDOMAIN} contains the local IP address of the Asterisk
server.
The userinfo arrives in ${CALLERID} and is displayed on the display of
the called device, and arrives in the CDR file.
Actually I do not know into which variable the incoming hostport info is
copied to?
Could somebody else answer this question?
Olle E Johansson wrote:
If I place a normal call to sip:ge...@example.com to my Asterisk server.
geert will be the extension I'm looking for, example.com ends up in
SIPDOMAIN. It's not the local IP address, it's the domain/host part of the
request URI in the INVITE.
I would prefer if phone context ended up in TELPHONECONTEXT so I could
use it the same way as SIPDOMAIN in the dial plan. It should not end up in
SIPDOMAIN as it is not a SIP uri. That way an extension in a local context
can be routed differently than an extension in a global context.
Just to make sure it's documented: The phone-context should NOT be matched to a
context in the dial plan. From a security standpoint, that would be horrific.
We need the information to be able to route correctly in the dial plan, but no
automatic selection. (I am not suggesting you where going down that path,
Geert. Just wanted to close that way of thinking since the word context could
lead there.)
- Olle E
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/#review11323
---
On March 22, 2014, 2:08 p.m., Geert Van Pamel wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/
---
(Updated March 22, 2014, 2:08 p.m.)
Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt
Jordan, and wdoekes.
Bugs: ASTERISK-17179
https://issues.asterisk.org/jira/browse/ASTERISK-17179
Repository: Asterisk
Description
---
Implements RFC-3966 TEL URI incoming INVITE.
See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description
of the original isssue.
I have been patching all versions since Asterisk 1.6. I would like to include
the code into the main trunk for version 13.
Previously Asterisk was failing with error on incoming IMS call:
Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address
missing 'sip:', using it anyway
Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a
SIP header (tel:0987654321;phone-context=+32987654321)?
Reason: tel: protocol was not recognized.
Diffs
-
/trunk/channels/sip/reqresp_parser.c 410429
/trunk/channels/chan_sip.c 410429
Diff: https://reviewboard.asterisk.org/r/3349/diff/
Testing
---
Executed an incoming TEL URI INVITE connection.
CLI was present on the display and in the CDR file.
No errors on SIP debug output.
File Attachments
RFC-3966 tel URI patch
https://reviewboard.asterisk.org/media/uploaded/files/2014/03/13/cad7a996-88c1-47fe-a2a9-cc6987af3b75__rfc-3966-tel-uri-patch-diff.txt
Thanks,
Geert Van Pamel
--
_
-- 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] 3375: [res_config_odbc/res_odbc] Fixed handling of non-text columns updates with empty values
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/ --- (Updated March 24, 2014, 9:31 a.m.) Review request for Asterisk Developers. Changes --- Minor cleanups. The check for text column moved to a separate inline function. Bugs: ASTERISK-23459 https://issues.asterisk.org/jira/browse/ASTERISK-23459 Repository: Asterisk Description --- This review request is a patch for the issue reviewed in https://reviewboard.asterisk.org/r/3335 but for non-trunk versions. Changes: - correct check for keyfield existence, - new res_odbc.conf variable allow_empty_string_in_nontext, - proper empty string = NULL conversion with allow_empty_string_in_nontext option disabled for non-text columns. This patch is based on branch 11 svn, but code for 1.8 and 10 is the same, so it should apply with no problems. Diffs (updated) - http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.exports.in 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/include/asterisk/res_odbc.h 411017 http://svn.asterisk.org/svn/asterisk/branches/11/configs/res_odbc.conf.sample 411017 Diff: https://reviewboard.asterisk.org/r/3375/diff/ Testing --- Tested with Asterisk 11 + ODBC + SIP realtime + PostgreSQL with both allow_empty_string_in_nontext settings to ensure no regression is introduced. Thanks, zvision -- _ -- 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] 3375: [res_config_odbc/res_odbc] Fixed handling of non-text columns updates with empty values
On March 23, 2014, 3:26 p.m., wdoekes wrote: http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c, line 533 https://reviewboard.asterisk.org/r/3375/diff/1/?file=56218#file56218line533 Please move the (non-inline) ast_odbc function call to the back so we can avoid the call for char-columns. Also, we don't need to check tableptr, since column would be NULL if it was false. I also moved the check for text column to a separate inline function. On March 23, 2014, 3:26 p.m., wdoekes wrote: http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c, lines 539-540 https://reviewboard.asterisk.org/r/3375/diff/1/?file=56218#file56218line539 + /* Value is non-zero, or column accepts empty strings, or we couldn't fit any more into cps.skip (count63 ?!). */ (Yes, that's the reason for the seemingly random 64.) Yes, it took me some time to figure out this mystery:) - zvision --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/#review11331 --- On March 24, 2014, 9:31 a.m., zvision wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/ --- (Updated March 24, 2014, 9:31 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23459 https://issues.asterisk.org/jira/browse/ASTERISK-23459 Repository: Asterisk Description --- This review request is a patch for the issue reviewed in https://reviewboard.asterisk.org/r/3335 but for non-trunk versions. Changes: - correct check for keyfield existence, - new res_odbc.conf variable allow_empty_string_in_nontext, - proper empty string = NULL conversion with allow_empty_string_in_nontext option disabled for non-text columns. This patch is based on branch 11 svn, but code for 1.8 and 10 is the same, so it should apply with no problems. Diffs - http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.exports.in 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/include/asterisk/res_odbc.h 411017 http://svn.asterisk.org/svn/asterisk/branches/11/configs/res_odbc.conf.sample 411017 Diff: https://reviewboard.asterisk.org/r/3375/diff/ Testing --- Tested with Asterisk 11 + ODBC + SIP realtime + PostgreSQL with both allow_empty_string_in_nontext settings to ensure no regression is introduced. Thanks, zvision -- _ -- 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] 3375: [res_config_odbc/res_odbc] Fixed handling of non-text columns updates with empty values
On March 23, 2014, 3:26 p.m., wdoekes wrote: http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c, line 533 https://reviewboard.asterisk.org/r/3375/diff/1/?file=56218#file56218line533 Please move the (non-inline) ast_odbc function call to the back so we can avoid the call for char-columns. Also, we don't need to check tableptr, since column would be NULL if it was false. zvision wrote: I also moved the check for text column to a separate inline function. The check for tableptr is neccessary when count 63. - zvision --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/#review11331 --- On March 24, 2014, 9:31 a.m., zvision wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/ --- (Updated March 24, 2014, 9:31 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23459 https://issues.asterisk.org/jira/browse/ASTERISK-23459 Repository: Asterisk Description --- This review request is a patch for the issue reviewed in https://reviewboard.asterisk.org/r/3335 but for non-trunk versions. Changes: - correct check for keyfield existence, - new res_odbc.conf variable allow_empty_string_in_nontext, - proper empty string = NULL conversion with allow_empty_string_in_nontext option disabled for non-text columns. This patch is based on branch 11 svn, but code for 1.8 and 10 is the same, so it should apply with no problems. Diffs - http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.exports.in 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/include/asterisk/res_odbc.h 411017 http://svn.asterisk.org/svn/asterisk/branches/11/configs/res_odbc.conf.sample 411017 Diff: https://reviewboard.asterisk.org/r/3375/diff/ Testing --- Tested with Asterisk 11 + ODBC + SIP realtime + PostgreSQL with both allow_empty_string_in_nontext settings to ensure no regression is introduced. Thanks, zvision -- _ -- 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] 3375: [res_config_odbc/res_odbc] Fixed handling of non-text columns updates with empty values
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/#review11344 --- Ship it! Looking good. - wdoekes On March 24, 2014, 9:52 a.m., zvision wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/ --- (Updated March 24, 2014, 9:52 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23459 https://issues.asterisk.org/jira/browse/ASTERISK-23459 Repository: Asterisk Description --- This review request is a patch for the issue reviewed in https://reviewboard.asterisk.org/r/3335 but for non-trunk versions. Changes: - correct check for keyfield existence, - new res_odbc.conf variable allow_empty_string_in_nontext, - proper empty string = NULL conversion with allow_empty_string_in_nontext option disabled for non-text columns. This patch is based on branch 11 svn, but code for 1.8 and 10 is the same, so it should apply with no problems. Diffs - http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.exports.in 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/include/asterisk/res_odbc.h 411017 http://svn.asterisk.org/svn/asterisk/branches/11/configs/res_odbc.conf.sample 411017 Diff: https://reviewboard.asterisk.org/r/3375/diff/ Testing --- Tested with Asterisk 11 + ODBC + SIP realtime + PostgreSQL with both allow_empty_string_in_nontext settings to ensure no regression is introduced. Thanks, zvision -- _ -- 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] 3377: ref count logs: Redo structure of log file, provide a python debugging tool
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3377/#review11345 --- /branches/1.8/main/astobj2.c https://reviewboard.asterisk.org/r/3377/#comment20982 int ref_debug is now unused. I think Corey found the rest, and more ;) - wdoekes On March 19, 2014, 6:22 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3377/ --- (Updated March 19, 2014, 6:22 p.m.) Review request for Asterisk Developers, Corey Farrell and wdoekes. Repository: Asterisk Description --- Note: while an improvement to Asterisk, this patch only affects Asterisk when compiled in -dev-mode. Since it has benefit only for developers looking to fix bugs, I'm proposing it for Asterisk 1.8+. The removal of refcounter should be done in trunk only. Asterisk uses reference counted objects. A lot. As their use has spread, the bugs related to reference counting errors has grown as well. Central to resolving reference counting issues is the usage of REF_DEBUG; unfortunately, REF_DEBUG has had several problems: (1) It could not be enabled through menuselect (2) When not enabled globally, updates to objects were often lost, resulting in insufficient data to resolve problems (3) The format of the ref debug log file was not suitable for parsing This patch changes REF_DEBUG in the following ways: (1) It makes REF_DEBUG a meneselect item when Asterisk is compiled with --enable-dev-mode (2) The ref debug log file is now created in the AST_LOG_DIR directory. Every run will now blow away the previous run (as large ref files sometimes caused issues). We now also no longer open/close the file on each write, instead relying on fflush to make sure data gets written to the file (in case the ao2 call being performed is about to cause a crash) (3) It goes with a comma delineated format for the ref debug file. This makes parsing much easier. (4) It throws out the refcounter utility and goes with a python script instead. Diffs - /branches/1.8/utils/refcounter.c 410891 /branches/1.8/utils/Makefile 410891 /branches/1.8/main/astobj2.c 410891 /branches/1.8/contrib/scripts/refcounter.py PRE-CREATION /branches/1.8/build_tools/cflags-devmode.xml 410891 Diff: https://reviewboard.asterisk.org/r/3377/diff/ Testing --- Things spit out ref logs and the script parses them. Example below: Object 0x21ed9b8 history features.c[5427]:create_parkinglot 1 - [**constructor**] features.c[5707]:build_parkinglot +1 - [1] features.c[5392]:parkinglot_unref -1 - [2] features.c[6358]:build_dialplan_useage_map +1 - [1] features.c[6358]:build_dialplan_useage_map -1 - [2] features.c[4985]:find_parkinglot +1 - [1] features.c[5392]:parkinglot_unref -1 - [2] features.c[6358]:build_dialplan_useage_map +1 - [1] features.c[6358]:build_dialplan_useage_map -1 - [2] features.c[4955]:do_parking_thread +1 - [1] features.c[4957]:do_parking_thread -1 - [2] astobj2.c[955]:cd_cb_debug -1 deref object via container destroy - [1] astobj2.c[955]:cd_cb_debug -1 deref object via container destroy - [**call destructor**] 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] 3375: [res_config_odbc/res_odbc] Fixed handling of non-text columns updates with empty values
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3375/ --- (Updated March 24, 2014, 10:14 a.m.) Review request for Asterisk Developers. Changes --- Redundant check for NULL column removed. Minor comment cleanup. Bugs: ASTERISK-23459 https://issues.asterisk.org/jira/browse/ASTERISK-23459 Repository: Asterisk Description --- This review request is a patch for the issue reviewed in https://reviewboard.asterisk.org/r/3335 but for non-trunk versions. Changes: - correct check for keyfield existence, - new res_odbc.conf variable allow_empty_string_in_nontext, - proper empty string = NULL conversion with allow_empty_string_in_nontext option disabled for non-text columns. This patch is based on branch 11 svn, but code for 1.8 and 10 is the same, so it should apply with no problems. Diffs (updated) - http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.exports.in 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c 411017 http://svn.asterisk.org/svn/asterisk/branches/11/include/asterisk/res_odbc.h 411017 http://svn.asterisk.org/svn/asterisk/branches/11/configs/res_odbc.conf.sample 411017 Diff: https://reviewboard.asterisk.org/r/3375/diff/ Testing --- Tested with Asterisk 11 + ODBC + SIP realtime + PostgreSQL with both allow_empty_string_in_nontext settings to ensure no regression is introduced. Thanks, zvision -- _ -- 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] 3349: Implement RFC-3966 TEL URI incoming INVITE
On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
I don't see what happens with the phone-context argument. Shouldn't we pass
that on as a channel variable?
Geert Van Pamel wrote:
We return this into the hostport.
Geert Van Pamel wrote:
According to RFC 3966 phone-context is either a domain-name, or (part of)
an international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the
local number must be unique within its context...
Olle E Johansson wrote:
So it ends up in the SIPDOMAIN variable in the dial plan? It has to be
reachable in the dial plan somehow.
Geert Van Pamel wrote:
The variable ${SIPDOMAIN} contains the local IP address of the Asterisk
server.
The userinfo arrives in ${CALLERID} and is displayed on the display of
the called device, and arrives in the CDR file.
Actually I do not know into which variable the incoming hostport info is
copied to?
Could somebody else answer this question?
Olle E Johansson wrote:
If I place a normal call to sip:ge...@example.com to my Asterisk server.
geert will be the extension I'm looking for, example.com ends up in
SIPDOMAIN. It's not the local IP address, it's the domain/host part of the
request URI in the INVITE.
I would prefer if phone context ended up in TELPHONECONTEXT so I could
use it the same way as SIPDOMAIN in the dial plan. It should not end up in
SIPDOMAIN as it is not a SIP uri. That way an extension in a local context
can be routed differently than an extension in a global context.
Olle E Johansson wrote:
Just to make sure it's documented: The phone-context should NOT be
matched to a context in the dial plan. From a security standpoint, that would
be horrific. We need the information to be able to route correctly in the
dial plan, but no automatic selection. (I am not suggesting you where going
down that path, Geert. Just wanted to close that way of thinking since the
word context could lead there.)
So I understand that Olle proposes to create a new variable ${TELPHONECONTEXT}
that contains the TEL URI phone-context which can be either the global number,
or a global number prefix, or the related routing domain or any other unique
routing identifier, or would be empty in case there is just a local number (as
specified in RFC 3966).
Currently this variable is not available yet in Asterisk. In the dialplan
treating incoming calls currently I do not use nor do not need this
information, as the local number in ${CALLERID} is sufficient (for the time
being)... Still this phone context identifier could be needed for subsequent
outgoing calls (return calls, callbacks, etc.).
I agree that ${SIPDOMAIN} will remain reserved for SIP invites, and is
untouched for TEL URI invites.
I perfectly understand that this TEL URI context has nothing to do with
dialplan context.
Who could us further advise how to create the new variable ${TELPHONECONTEXT} ?
- Geert
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/#review11323
---
On March 22, 2014, 2:08 p.m., Geert Van Pamel wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/
---
(Updated March 22, 2014, 2:08 p.m.)
Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt
Jordan, and wdoekes.
Bugs: ASTERISK-17179
https://issues.asterisk.org/jira/browse/ASTERISK-17179
Repository: Asterisk
Description
---
Implements RFC-3966 TEL URI incoming INVITE.
See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description
of the original isssue.
I have been patching all versions since Asterisk 1.6. I would like to include
the code into the main trunk for version 13.
Previously Asterisk was failing with error on incoming IMS call:
Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address
missing 'sip:', using it anyway
Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a
SIP header (tel:0987654321;phone-context=+32987654321)?
Reason: tel: protocol was not recognized.
Diffs
-
/trunk/channels/sip/reqresp_parser.c 410429
/trunk/channels/chan_sip.c 410429
Diff: https://reviewboard.asterisk.org/r/3349/diff/
Testing
---
Executed an incoming TEL URI INVITE connection.
CLI was present on the display and in the CDR file.
No errors on SIP debug output.
File Attachments
RFC-3966 tel URI patch
Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE
On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
I don't see what happens with the phone-context argument. Shouldn't we pass
that on as a channel variable?
Geert Van Pamel wrote:
We return this into the hostport.
Geert Van Pamel wrote:
According to RFC 3966 phone-context is either a domain-name, or (part of)
an international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the
local number must be unique within its context...
Olle E Johansson wrote:
So it ends up in the SIPDOMAIN variable in the dial plan? It has to be
reachable in the dial plan somehow.
Geert Van Pamel wrote:
The variable ${SIPDOMAIN} contains the local IP address of the Asterisk
server.
The userinfo arrives in ${CALLERID} and is displayed on the display of
the called device, and arrives in the CDR file.
Actually I do not know into which variable the incoming hostport info is
copied to?
Could somebody else answer this question?
Olle E Johansson wrote:
If I place a normal call to sip:ge...@example.com to my Asterisk server.
geert will be the extension I'm looking for, example.com ends up in
SIPDOMAIN. It's not the local IP address, it's the domain/host part of the
request URI in the INVITE.
I would prefer if phone context ended up in TELPHONECONTEXT so I could
use it the same way as SIPDOMAIN in the dial plan. It should not end up in
SIPDOMAIN as it is not a SIP uri. That way an extension in a local context
can be routed differently than an extension in a global context.
Olle E Johansson wrote:
Just to make sure it's documented: The phone-context should NOT be
matched to a context in the dial plan. From a security standpoint, that would
be horrific. We need the information to be able to route correctly in the
dial plan, but no automatic selection. (I am not suggesting you where going
down that path, Geert. Just wanted to close that way of thinking since the
word context could lead there.)
Geert Van Pamel wrote:
So I understand that Olle proposes to create a new variable
${TELPHONECONTEXT} that contains the TEL URI phone-context which can be
either the global number, or a global number prefix, or the related routing
domain or any other unique routing identifier, or would be empty in case
there is just a local number (as specified in RFC 3966).
Currently this variable is not available yet in Asterisk. In the dialplan
treating incoming calls currently I do not use nor do not need this
information, as the local number in ${CALLERID} is sufficient (for the time
being)... Still this phone context identifier could be needed for subsequent
outgoing calls (return calls, callbacks, etc.).
I agree that ${SIPDOMAIN} will remain reserved for SIP invites, and is
untouched for TEL URI invites.
I perfectly understand that this TEL URI context has nothing to do with
dialplan context.
Who could us further advise how to create the new variable
${TELPHONECONTEXT} ?
Just grep for SIPDOMAIN in the chan_sip source code :-)
- Olle E
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/#review11323
---
On March 22, 2014, 2:08 p.m., Geert Van Pamel wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/
---
(Updated March 22, 2014, 2:08 p.m.)
Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt
Jordan, and wdoekes.
Bugs: ASTERISK-17179
https://issues.asterisk.org/jira/browse/ASTERISK-17179
Repository: Asterisk
Description
---
Implements RFC-3966 TEL URI incoming INVITE.
See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description
of the original isssue.
I have been patching all versions since Asterisk 1.6. I would like to include
the code into the main trunk for version 13.
Previously Asterisk was failing with error on incoming IMS call:
Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address
missing 'sip:', using it anyway
Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a
SIP header (tel:0987654321;phone-context=+32987654321)?
Reason: tel: protocol was not recognized.
Diffs
-
/trunk/channels/sip/reqresp_parser.c 410429
/trunk/channels/chan_sip.c 410429
Diff: https://reviewboard.asterisk.org/r/3349/diff/
Testing
---
Executed an incoming TEL URI INVITE connection.
CLI was present on the display and in the CDR file.
No errors on SIP debug output.
File Attachments
Re: [asterisk-dev] [Code Review] 3380: ARI: When a channel controlled by stasis is removed from a bridge, automatically created subscriptions to the bridge aren't removed.
On March 21, 2014, 2:05 p.m., Jonathan Rose wrote: /branches/12/res/res_stasis.c, line 739 https://reviewboard.asterisk.org/r/3380/diff/1/?file=56309#file56309line739 It might also be appropriate to move this declaration into the loop... but ultimately kind of pointless since it gets reassigned right after the hangup check anyway. Go ahead and move this into the loop since it doesn't need to persist beyond the loop or across iterations. - opticron --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3380/#review11316 --- On March 21, 2014, 2:03 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3380/ --- (Updated March 21, 2014, 2:03 p.m.) Review request for Asterisk Developers, Matt Jordan and opticron. Repository: Asterisk Description --- I was examining a testsuite failure (which should be resolved by https://reviewboard.asterisk.org/r/3361/) and stumbled across the following behavior: A channel enters a stasis application ARI pushes that channel into a bridge - the bridge is automatically subscribed to the stasis application ARI pulls the channel from the bridge - the bridge is still subscribed to the stasis application. Looking into why this was, I became aware that the patch which introduced the aforementioned test failure (which turned out to be just because the test was trying to unsubscribe for something that it probably shouldn't have been trying to unsubscribe) had introduced a two subscription scheme when the stasis channels are added to bridges and that only one of these stasis channels was being cleared. It turned out to be a rather simple problem where the previously used bridge can't be tracked properly because we are resetting it to NULL all the time. The fix was quite simple and I just had to move the initialization of the bridge used to track through repeat iterations out of the loop. Today's lesson: Hooray for accidentally looking into test failures that have already been solved! Diffs - /branches/12/res/res_stasis.c 411008 Diff: https://reviewboard.asterisk.org/r/3380/diff/ Testing --- Pretty much just used the scenario described above twice. The test still fails with this patch in place simply because the second subscription is still active as the channel exits the bridge and deleting automatically created subscriptions manually is probably just a bad idea in general. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3354: Test Suite: MWI subscribe, re-subscribe, and un-subscribe test for PJSIP
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3354/ --- (Updated March 24, 2014, 9:14 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 4886 Bugs: ASTERISK-23343 https://issues.asterisk.org/jira/browse/ASTERISK-23343 Repository: testsuite Description --- This basic nominal test ensures that a mailbox on an AOR for an endpoint can be subscribed to for MWI, re-subscribe, and un-subscribe. It ensures the Event and Subscription-State headers of the NOTIFY are what is expected. This takes care the first two tests listed on ASTERISK-23343 and uses SIPp instead of the PJSUA library as used for the 3rd test up for review at https://reviewboard.asterisk.org/r/3348/ Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/sub_setup_teardown/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/sub_setup_teardown/sipp/mwi_subscription.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/sub_setup_teardown/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/sub_setup_teardown/configs/ast1/modules.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3354/diff/ Testing --- * Ensured tests pass on multiple executions * Ensured the testsuite Asterisk logs looked good. 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
Re: [asterisk-dev] [Code Review] 3381: res_pjsip: Fix contact authenticate_qualify endpoint lookup when qualifing a contact.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3381/#review11350 --- Ship it! Looks good. No issue, just something you might have already checked for. /branches/12/res/res_pjsip.c https://reviewboard.asterisk.org/r/3381/#comment20985 Just to make sure did you check every call to this to make sure it wasn't being decremented by the caller? - Kevin Harwell On March 21, 2014, 8:16 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3381/ --- (Updated March 21, 2014, 8:16 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23254 https://issues.asterisk.org/jira/browse/ASTERISK-23254 Repository: Asterisk Description --- * Fixed bad use of ao2_find() in on_endpoint(). * Replaced use of find_endpoints() with find_an_endpoint() since only the first found endpoint is ever needed. * Fixed qualify_contact_cb() to update the contact with the aor authenticate_qualify setting. Otherwise, permanent contacts in the aor type sections would have a config line order dependancy. * Fixed off nominal path contact ref leak in qualify_contact(). The comment saying the unref is not needed was wrong. * Fixed off nominal path use of the endpoint parameter if it is NULL in send_out_of_dialog_request(). * Added missing off nominal path unref of pjsip tdata in send_out_of_dialog_request(). * Fixed off nominal path failing to call the callback in send_request_cb() when the request is challenged for authentication. * Eliminated silly RAII_VAR() use in qualify_contact_cb(). * Updated ast_sip_send_request() doxygen to better reflect reality. Diffs - /branches/12/res/res_pjsip/pjsip_options.c 411010 /branches/12/res/res_pjsip.c 411010 /branches/12/include/asterisk/res_pjsip.h 411010 Diff: https://reviewboard.asterisk.org/r/3381/diff/ Testing --- Added a testing debug message to indicate when an endpoint was found. An endpoint was found when expected. However, I ran into several config reload problems which couldn't be readily fixed that made reloading the aor options not work. 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] 3381: res_pjsip: Fix contact authenticate_qualify endpoint lookup when qualifing a contact.
On March 24, 2014, 11:32 a.m., Kevin Harwell wrote: /branches/12/res/res_pjsip.c, lines 1872-1876 https://reviewboard.asterisk.org/r/3381/diff/2/?file=56314#file56314line1872 Just to make sure did you check every call to this to make sure it wasn't being decremented by the caller? I did check and I just checked it again. This is only called by one place: ast_sip_send_request(). The callers of ast_sip_send_request() do not release tdata. Several don't even check if there was an error return. Also the pjsip stack is expecting the tdata ref to be passed to it rather than the caller still owning the ref. Since this is an error condition before passing tdata to the pjsip stack this routine must release the tdata ref. Otherwise the routine would be inconsistent in who should release the ref. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3381/#review11350 --- On March 21, 2014, 8:16 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3381/ --- (Updated March 21, 2014, 8:16 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23254 https://issues.asterisk.org/jira/browse/ASTERISK-23254 Repository: Asterisk Description --- * Fixed bad use of ao2_find() in on_endpoint(). * Replaced use of find_endpoints() with find_an_endpoint() since only the first found endpoint is ever needed. * Fixed qualify_contact_cb() to update the contact with the aor authenticate_qualify setting. Otherwise, permanent contacts in the aor type sections would have a config line order dependancy. * Fixed off nominal path contact ref leak in qualify_contact(). The comment saying the unref is not needed was wrong. * Fixed off nominal path use of the endpoint parameter if it is NULL in send_out_of_dialog_request(). * Added missing off nominal path unref of pjsip tdata in send_out_of_dialog_request(). * Fixed off nominal path failing to call the callback in send_request_cb() when the request is challenged for authentication. * Eliminated silly RAII_VAR() use in qualify_contact_cb(). * Updated ast_sip_send_request() doxygen to better reflect reality. Diffs - /branches/12/res/res_pjsip/pjsip_options.c 411010 /branches/12/res/res_pjsip.c 411010 /branches/12/include/asterisk/res_pjsip.h 411010 Diff: https://reviewboard.asterisk.org/r/3381/diff/ Testing --- Added a testing debug message to indicate when an endpoint was found. An endpoint was found when expected. However, I ran into several config reload problems which couldn't be readily fixed that made reloading the aor options not work. 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] 3382: RFC: Weak Reference Containers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3382/ --- (Updated March 24, 2014, 11:17 a.m.) Review request for Asterisk Developers, Joshua Colp and rmudgett. Changes --- Added a 'destroying' flag to astobj2 that gets set immediately after internal_ao2_ref recognizes that an object is being destroyed. Subsequent hash and rbtree find/iterate functions will skip that node as though it were an empty node. I think this addresses Corey's concern over the race condition. Repository: Asterisk Description --- This is a Request For Comments patch that adds weak reference capability to astobj2 containers. Current (Strong-ref) behavior: A container (list, hash or rbtree) increments an object's ref count when linked and decrements an object's ref count when unlinked. Therefore the object can never be destroyed while it is an entry in a container unless someone accidentally decrements the object's ref count too many times. In this case, the object is invalid but the container doesn't know it. Weak-ref behavior: A container (list, hash or rbtree) allocated with the option AO2_CONTAINER_ALLOC_OPT_WEAK_REF neither increments an object's ref count when linked nor decrements an object's ref count when unlinked. If the object's ref count can therefore validly reach 0 in which case the object is automatically and cleanly removed from any weak-ref container it may be in. Use case: The possible need for weak-ref containers came up during the development of the Sorcery registry. The first call to sorcery_open from a module should create a new sorcery instance and subsequent calls from that same module should use that instance. The last call to close should free that instance. With a strong-ref container however, the container itself always has a a reference to the instance so it doesn't get destroyed without special code to check the ref count on each call to close. Implementation: astobj2.priv_data now has a linked list that contains the weak-ref container nodes that reference the object. When an object is added to a weak-ref container, the container node is added to the list instead of the node incrementing the object's ref count. Similarly, when an object is removed from a weak-ref container the node is removed from the linked list instead of the object's ref count being decremented. If an object's ref count reaches 0 the object's linked list is traversed and the corresponding nodes are cleared effectively removing the object from the container. NOTE: An object's ref count is still incremented as the result of a retrieval (find, iterate, etc) from a weak-ref container. Backwards compatibility: Unless the AO2_CONTAINER_ALLOC_OPT_WEAK_REF flag was set on container allocation, all container operations remain exactly as they are today and nothing prevents an object from being a member of both strong and weak ref containers at the same time. Performance implications: Due to code reorganization, the performance of strong-ref containers is actually microscopically BETTER than the unmodified code and the performance of weak-ref containers is even better than that. In other words, the performance of the default behavior was not penalized by the addition of the new feature. Code reorganization. I moved all of the structure definitions in astobj2.c to astobj2_private.h. This makes astobj2.c much easier to read and debug. I was also able to push down some implementation specific code to the hash and rbtree functions and pull up some duplicated code from the hash and rbtree functions. Patch notes: The patch file itself might be a little hard to read because of the reorganization so applying the patch is your best bet for detailed review. The patch will apply cleanly to both branches/12 and trunk. Also, the patch disables AO2_DEBUG and AST_DEVMODE in astobj2 so that performance can be tested while still keeping the test framework. The final patch will remove the disables. Diffs (updated) - branches/12/tests/test_astobj2_thrash.c 411013 branches/12/tests/test_astobj2.c 411013 branches/12/main/astobj2.c 411013 branches/12/include/asterisk/astobj2.h 411013 Diff: https://reviewboard.asterisk.org/r/3382/diff/ Testing --- All of the existing unit tests in test_astobj2 pass including the thrash test. I added 7 additional unit tests specifically for the weak-ref implementation including a performance comparison test that compares both strong and weak ref implementations. A thrash test was also added for weak-ref. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit:
[asterisk-dev] [Code Review] 3383: TestSuite: Fix bouncing show_subscriptions test
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3383/
---
Review request for Asterisk Developers.
Repository: testsuite
Description
---
This fixes a race condition in the tests/channels/pjsip/ami/show_subscriptions
test where the PJSIPShowSubscriptionsInbound AMI command would be issued before
states had finished transitioning resulting in the response containing NULL
instead of ACTIVE. This also requires the following patch that adds a test
event when Asterisk is compiled in development mode:
Index: res/res_pjsip_pubsub.c
===
--- res/res_pjsip_pubsub.c (revision 410980)
+++ res/res_pjsip_pubsub.c (working copy)
@@ -42,6 +42,7 @@
#include asterisk/res_pjsip.h
#include asterisk/callerid.h
#include asterisk/manager.h
+#include asterisk/test.h
#include res_pjsip/include/res_pjsip_private.h
/*** DOCUMENTATION
@@ -464,8 +465,18 @@
int ast_sip_subscription_send_request(struct ast_sip_subscription *sub,
pjsip_tx_data *tdata)
{
- return pjsip_evsub_send_request(ast_sip_subscription_get_evsub(sub),
+ struct ast_sip_endpoint *endpoint =
ast_sip_subscription_get_endpoint(sub);
+ int res = pjsip_evsub_send_request(ast_sip_subscription_get_evsub(sub),
tdata) == PJ_SUCCESS ? 0 : -1;
+
+ ast_test_suite_event_notify(SUBSCRIPTION_STATE_SET,
+ StateText: %s\r\n
+ Endpoint: %s\r\n,
+ pjsip_evsub_get_state_name(ast_sip_subscription_get_evsub(sub)),
+ ast_sorcery_object_get_id(endpoint));
+
+ ao2_cleanup(endpoint);
+ return res;
}
static void subscription_datastore_destroy(void *obj)
Diffs
-
asterisk/trunk/tests/channels/pjsip/ami/show_subscriptions/AMISendTest.py
4885
Diff: https://reviewboard.asterisk.org/r/3383/diff/
Testing
---
Verified that the race was resolved by using state transition test events
instead of the successful callback of the SIPp scenario.
Thanks,
opticron
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3377: ref count logs: Redo structure of log file, provide a python debugging tool
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3377/#review11352 --- /branches/1.8/utils/Makefile https://reviewboard.asterisk.org/r/3377/#comment20987 Removing refcounter from the cleanup should only be done on trunk if anywhere. Otherwise, you could have a stale refcounter program lying around after an svn up. Alternatively the refcounter could be moved to is own rm line and commented as removing legacy build objects. - rmudgett On March 19, 2014, 1:22 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3377/ --- (Updated March 19, 2014, 1:22 p.m.) Review request for Asterisk Developers, Corey Farrell and wdoekes. Repository: Asterisk Description --- Note: while an improvement to Asterisk, this patch only affects Asterisk when compiled in -dev-mode. Since it has benefit only for developers looking to fix bugs, I'm proposing it for Asterisk 1.8+. The removal of refcounter should be done in trunk only. Asterisk uses reference counted objects. A lot. As their use has spread, the bugs related to reference counting errors has grown as well. Central to resolving reference counting issues is the usage of REF_DEBUG; unfortunately, REF_DEBUG has had several problems: (1) It could not be enabled through menuselect (2) When not enabled globally, updates to objects were often lost, resulting in insufficient data to resolve problems (3) The format of the ref debug log file was not suitable for parsing This patch changes REF_DEBUG in the following ways: (1) It makes REF_DEBUG a meneselect item when Asterisk is compiled with --enable-dev-mode (2) The ref debug log file is now created in the AST_LOG_DIR directory. Every run will now blow away the previous run (as large ref files sometimes caused issues). We now also no longer open/close the file on each write, instead relying on fflush to make sure data gets written to the file (in case the ao2 call being performed is about to cause a crash) (3) It goes with a comma delineated format for the ref debug file. This makes parsing much easier. (4) It throws out the refcounter utility and goes with a python script instead. Diffs - /branches/1.8/utils/refcounter.c 410891 /branches/1.8/utils/Makefile 410891 /branches/1.8/main/astobj2.c 410891 /branches/1.8/contrib/scripts/refcounter.py PRE-CREATION /branches/1.8/build_tools/cflags-devmode.xml 410891 Diff: https://reviewboard.asterisk.org/r/3377/diff/ Testing --- Things spit out ref logs and the script parses them. Example below: Object 0x21ed9b8 history features.c[5427]:create_parkinglot 1 - [**constructor**] features.c[5707]:build_parkinglot +1 - [1] features.c[5392]:parkinglot_unref -1 - [2] features.c[6358]:build_dialplan_useage_map +1 - [1] features.c[6358]:build_dialplan_useage_map -1 - [2] features.c[4985]:find_parkinglot +1 - [1] features.c[5392]:parkinglot_unref -1 - [2] features.c[6358]:build_dialplan_useage_map +1 - [1] features.c[6358]:build_dialplan_useage_map -1 - [2] features.c[4955]:do_parking_thread +1 - [1] features.c[4957]:do_parking_thread -1 - [2] astobj2.c[955]:cd_cb_debug -1 deref object via container destroy - [1] astobj2.c[955]:cd_cb_debug -1 deref object via container destroy - [**call destructor**] 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] 3337: Code for DTLS retransmission
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3337/#review11353 --- http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c https://reviewboard.asterisk.org/r/3337/#comment20988 Coding guidelines: Opening brace goes on the same line as the if. http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c https://reviewboard.asterisk.org/r/3337/#comment20991 There is a race condition here where multiple timeouts could be setup if the timer callback is running when dtls_srtp_check_pending is called from outside the timer callback. When this happens, the AST_SCHED_DEL_UNREF attempt will instead have to use ast_sched_del to check for failure and abort rescheduling for one of the call paths (timer-dtls_srtp_check_pending or other_code-dtls_srtp_check_pending) since the other code path will need to go ahead with the rescheduling attempt. http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c https://reviewboard.asterisk.org/r/3337/#comment20992 Coding guidelines. http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c https://reviewboard.asterisk.org/r/3337/#comment20993 Coding guidelines. http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c https://reviewboard.asterisk.org/r/3337/#comment20994 Coding guidelines. - opticron On March 12, 2014, 8:21 a.m., Nitesh Bansal wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3337/ --- (Updated March 12, 2014, 8:21 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This patch adds the code to do the DTLS retransmissions in Asterisk. Diffs - http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c 410469 Diff: https://reviewboard.asterisk.org/r/3337/diff/ Testing --- I tested this with a basic SIPP script, which fakes a DTLS INVITE. Asterisk thinks that it is a DTLS call and inititates the DTLS handshake. SIPP doesn't respond to DTLS handshake, which causes the DTLS timeout and DTLS retransmission takes place. Thanks, Nitesh Bansal -- _ -- 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] 3384: PJSIP: Add 'message_context' configuration option.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3384/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- While doing experimentation, I realized that there is no configuration option that allows for incoming MESSAGE requests to be routed to their own dialplan context. This adds a simple message_context option for endpoints that allow for a MESSAGE-specific context to be used. If a message_context is not provided, then the context setting is used for incoming MESSAGE requests from the endpoint. One thing that is conspicuously missing from this review request is an alembic revision script for this new option. Unfortunately, due to the unresolved issue of having multiple heads, I currently cannot provide a revision script. Diffs - /branches/12/res/res_pjsip_messaging.c 411020 /branches/12/res/res_pjsip/pjsip_configuration.c 411020 /branches/12/res/res_pjsip.c 411020 /branches/12/include/asterisk/res_pjsip.h 411020 Diff: https://reviewboard.asterisk.org/r/3384/diff/ Testing --- See /r/3385 Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3385: Testsuite: Add simple test for message_context PJSIP endpoint option.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3385/ --- Review request for Asterisk Developers. Repository: testsuite Description --- This tests the functionality introduced in /r/3384 This is a simple SIPp test that ensures that incoming MESSAGE requests are routed where they are expected to go. One endpoint has a message_context configured, but the other does not. UserEvents are checked to ensure the MESSAGEs were routed properly. Diffs - /asterisk/trunk/tests/channels/pjsip/message/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/message/message_context/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/message/message_context/sipp/message.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/message/message_context/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/message/message_context/configs/ast1/modules.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/message/message_context/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3385/diff/ Testing --- Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3384: PJSIP: Add 'message_context' configuration option.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3384/#review11357 --- Ship it! Ship It! - Joshua Colp On March 24, 2014, 7:31 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3384/ --- (Updated March 24, 2014, 7:31 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- While doing experimentation, I realized that there is no configuration option that allows for incoming MESSAGE requests to be routed to their own dialplan context. This adds a simple message_context option for endpoints that allow for a MESSAGE-specific context to be used. If a message_context is not provided, then the context setting is used for incoming MESSAGE requests from the endpoint. One thing that is conspicuously missing from this review request is an alembic revision script for this new option. Unfortunately, due to the unresolved issue of having multiple heads, I currently cannot provide a revision script. Diffs - /branches/12/res/res_pjsip_messaging.c 411020 /branches/12/res/res_pjsip/pjsip_configuration.c 411020 /branches/12/res/res_pjsip.c 411020 /branches/12/include/asterisk/res_pjsip.h 411020 Diff: https://reviewboard.asterisk.org/r/3384/diff/ Testing --- See /r/3385 Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3357: testsuite: Add off-nominal subscription tests for PJSIP.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3357/#review11358 --- Four of the five tests presented here (no_event_header, below_min_expiry, unallowed, and unknown_event_package) should use the SIPp test case instead of copying and pasting the same run-test script for each one. This requires adding ~10 lines of yaml to test-config.yaml for each test and allows for run-test to be completely deleted for those tests. It's a good tradeoff. The no_accept_header could also be mostly yaml-driven, but the use of a run-test isn't quite as offensive there. I think a no_accept_header test should be written for all event packages we support. For now that just means writing one for MWI. I'll leave the decision as to whether that should be done now as part of this work or whether that should have its own issue created to Matt. /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/sipp/subscribe.xml https://reviewboard.asterisk.org/r/3357/#comment21006 You have some problematic XML in this line here. I suggest changing the backslash-escaped double-quotes to quot; and the in the regex to g t ; Sorry, I couldn't put those characters together because reviewboard actually decodes the string into the the symbol :) /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/tests.yaml https://reviewboard.asterisk.org/r/3357/#comment21007 unallowed and below_min_expiry don't have anything to do with presence in particular, so they shouldn't be in the presence subdirectory. - Mark Michelson On March 14, 2014, 7:13 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3357/ --- (Updated March 14, 2014, 7:13 p.m.) Review request for Asterisk Developers, Kevin Harwell and Matt Jordan. Bugs: ASTERISK-23342 https://issues.asterisk.org/jira/browse/ASTERISK-23342 Repository: testsuite Description --- No Accept header This would set up the subscription, but use the default type for the event package being subscribed for Disallowed subscriptions A SIP UA subscribes for a valid event package with Asterisk, but the endpoint doesn't allow subscriptions Asterisk responds with a 603 MinExpiry not met A SIP UA sends a subscription with an expiration time that is less than the configured minexpiry for the endpoint Asterisk responds with a 423 No Event Header A SIP UA sends a subscription but fails to provide an Event header Asterisk responds with a 489 Unknown Event Package A SIP UA sends a subscription for an unknown event package Asterisk responds with a 489 Each of these tests is based on kharwell's Digium Presence test. As such, the No Accept Header test does require some digium phone specific stuff to be loaded in order to work. For all the other tests though, the tests are fairly general and will just fail for the reasons you would expect. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/run-test PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/unallowed/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/unallowed/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/unallowed/run-test PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/unallowed/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/unallowed/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/run-test PRE-CREATION
Re: [asterisk-dev] [Code Review] 3382: RFC: Weak Reference Containers
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3382/#review11355
---
* I think you need to add an assert to prevent ao2 objects that don't have a
lock from participating in weak ref containers. Otherwise, the object's weak
ref list has no reentrancy protection.
* Container node destructors assume that the container is already write locked
because the destructor unlinks the node from the container's structure.
* Red-black trees are not handling the weak ref nodes that have a destroying
object very well. The container could still use the key to determine which
child to go down for searches since the object still exists in the container.
The container just cannot return the object. Inserting duplicate objects could
consider the destroying object still in the tree.
* The AO2_CONTAINER_ALLOC_OPT_DUPS_REPLACE case where the found node is for a
destroying object needs examining for red-black trees. In this case I think
that the insert needs to consider the node as not a match and continue looking.
* The AO2_CONTAINER_ALLOC_OPT_DUPS_REPLACE needs to update the old linked
object's weak-ref list when another object replaces it. Weak-ref containers
probably need to treat replace as a delete followed by an insert. It may be
best to not allow it because destroying objects make the replace optimization
of assuming one and only one object with that key exists no longer valid.
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment21001
Curly on the same line as AST_LIST_TRAVERSE.
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment21000
The node's container must be write locked before unreffing here in case
this is the last reference of the container's node.
The container node normally has one reference (the container itself). The
other times the container node is referenced is when an iterator is currently
at that node in its iteration.
The object's weak ref list may need to have a ref to the container to
prevent a race between the container destruction and the object destruction.
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment21003
tag, file, line, and func need to be passed in to give credit in user code
where an object was linked into the container.
Probably need to add tag, file, line, and func parameters to
internal_ao2_unlink_node() for the same reason.
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment20998
You need to treat the ao2 objects as if they had rwlocks and use
ao2_wrlock() instead of ao2_lock(). Using ao2_wrlock() over ao2_lock() is
really a formality since ao2_lock() means ao2_wrlock().
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment21002
Add a note:
The node's container must be write locked before calling if the
AO2_UNLINK_NODE_UNREF_NODE flag is passed in.
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment20990
Just use AST_LIST_REMOVE() here.
AST_LIST_REMOVE() does its own traverse to find the node in the list to
remove it. You don't need an explicit AST_LIST_TRAVERSE().
branches/12/main/astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment20999
The case lines are indented too far.
branches/12/tests/test_astobj2.c
https://reviewboard.asterisk.org/r/3382/#comment20996
Guidelines:
These mostly appear to be copy-paste repetition:
switch (type) {
while () {
case TEST_CONTAINER_HASH:
- rmudgett
On March 24, 2014, 12:17 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3382/
---
(Updated March 24, 2014, 12:17 p.m.)
Review request for Asterisk Developers, Joshua Colp and rmudgett.
Repository: Asterisk
Description
---
This is a Request For Comments patch that adds weak reference capability to
astobj2 containers.
Current (Strong-ref) behavior: A container (list, hash or rbtree) increments
an object's ref count when linked and decrements an object's ref count when
unlinked. Therefore the object can never be destroyed while it is an entry
in a container unless someone accidentally decrements the object's ref count
too many times. In this case, the object is invalid but the container
doesn't know it.
Weak-ref behavior: A container (list, hash or rbtree) allocated with the
option AO2_CONTAINER_ALLOC_OPT_WEAK_REF neither increments an object's ref
count when linked nor decrements an object's ref count when unlinked. If the
object's ref count can therefore validly reach 0 in
Re: [asterisk-dev] [Code Review] 3374: testsuite: Verify that each of three users entering a conference enters muted when using the startmuted user profile option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3374/#review11359 --- What are the files in the configs/ast2/ directory used for? /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/test-config.yaml https://reviewboard.asterisk.org/r/3374/#comment21008 I suggest organizing these events into their respective channels sections. - Mark Michelson On March 18, 2014, 7:54 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3374/ --- (Updated March 18, 2014, 7:54 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23461 https://issues.asterisk.org/jira/browse/ASTERISK-23461 Repository: testsuite Description --- This relies on the addition of a test event in the conf_update_user_mute function: ast_test_suite_event_notify(CONF_MUTE_UPDATE, Mode: %s\r\n Conference: %s\r\n Channel: %s, mute_effective ? muted : unmuted, user-b_profile.name, ast_channel_name(user-chan)); The test is pretty simple. Three users enter a conference one after another while using a user profile with the startmuted option set. When all of the users have entered, the test is shut down. If any of those users' channels didn't receive the CONF_MUTE_UPDATE event or if the Mode for that event isn't muted, the test is failed. Diffs - /asterisk/trunk/tests/apps/confbridge/tests.yaml 4872 /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast2/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/manager.general.conf.inc PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/confbridge.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3374/diff/ Testing --- Ran the test, confirmed the events, set the expectations of some things to intentional failures to verify that it would fail if mismatches occur. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3353: testsuite: Test for receiving Play/Record start and stop events on ARI bridge playback/recording.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3353/#review11360 --- In retrospect, since the bridge is automatically subscribed by the channel being pushed into it, there is no need to maintain a separate application at all. I'm revising the test to eliminate it. - Jonathan Rose On March 13, 2014, 5:47 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3353/ --- (Updated March 13, 2014, 5:47 p.m.) Review request for Asterisk Developers, Matt Jordan and Mark Michelson. Bugs: ASTERISK-23444 https://issues.asterisk.org/jira/browse/ASTERISK-23444 Repository: testsuite Description --- As mmichelson suggested, here are a couple automated tests for replicating the tests I did by hand in https://reviewboard.asterisk.org/r/3340/ The mechanism is fairly simple... Channel enters stasis and starts the whole process Mixing bridge gets created via ARI An application subscribes to bridge stasis messages for the newly created bridge Channel gets pushed into the bridge via ARI Bridge is recorded via ARI / Bridge playback with sound:tt-weasels is executed via ARI The sound comes naturally to an end / the recording is stopped manually upon receiving its startup event The test closes in the same way as a normal bridge subscription test Diffs - /asterisk/trunk/tests/rest_api/bridges/tests.yaml 4836 /asterisk/trunk/tests/rest_api/bridges/bridge_record/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_record/subscribe_bridge.py PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_record/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/subscribe_bridge.py PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3353/diff/ Testing --- ran both tests some number of times to ensure success was repeatable. Varied some expectations to make sure everything I was looking for was actually being checked appropriately. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3353: testsuite: Test for receiving Play/Record start and stop events on ARI bridge playback/recording.
On March 20, 2014, 8:17 a.m., opticron wrote: /asterisk/trunk/tests/rest_api/bridges/bridge_record/subscribe_bridge.py, line 46 https://reviewboard.asterisk.org/r/3353/diff/1/?file=55987#file55987line46 Use the logger instead if this needs to be logged somewhere. Just leftover garbage. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3353/#review11293 --- On March 13, 2014, 5:47 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3353/ --- (Updated March 13, 2014, 5:47 p.m.) Review request for Asterisk Developers, Matt Jordan and Mark Michelson. Bugs: ASTERISK-23444 https://issues.asterisk.org/jira/browse/ASTERISK-23444 Repository: testsuite Description --- As mmichelson suggested, here are a couple automated tests for replicating the tests I did by hand in https://reviewboard.asterisk.org/r/3340/ The mechanism is fairly simple... Channel enters stasis and starts the whole process Mixing bridge gets created via ARI An application subscribes to bridge stasis messages for the newly created bridge Channel gets pushed into the bridge via ARI Bridge is recorded via ARI / Bridge playback with sound:tt-weasels is executed via ARI The sound comes naturally to an end / the recording is stopped manually upon receiving its startup event The test closes in the same way as a normal bridge subscription test Diffs - /asterisk/trunk/tests/rest_api/bridges/tests.yaml 4836 /asterisk/trunk/tests/rest_api/bridges/bridge_record/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_record/subscribe_bridge.py PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_record/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/subscribe_bridge.py PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3353/diff/ Testing --- ran both tests some number of times to ensure success was repeatable. Varied some expectations to make sure everything I was looking for was actually being checked appropriately. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3326: Sorcery: Do not apply the same wizard to an object type twice; Automatically apply sorcery configuration when sorcery is opened.
On March 20, 2014, 10:01 p.m., rmudgett wrote: /branches/12/main/sorcery.c, lines 777-779 https://reviewboard.asterisk.org/r/3326/diff/5/?file=56181#file56181line777 Should a duplicate wizard be considered a failure here? If so we should: res = sorcery_apply_wizard_mapping() if (res != success) break No, a duplicate wizard isn't a failure here. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3326/#review11309 --- On March 17, 2014, 8:31 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3326/ --- (Updated March 17, 2014, 8:31 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When performing some realtime tests, I noticed that the AMI command PJSIPShowEndpoints was listing all of my endpoints twice. This is because ast_sorcery_apply_config() was being called twice from res_pjsip code, once when initializing system configuration, and once again when initializing the rest of the configuration data. This patch aims to fix the problem on two fronts: 1) Remove the ast_sorcery_apply_config() calls from the PJSIP code entirely in favor of having sorcery automatically apply configuration for the module when sorcery is opened. This reduces the chance of accidentally attempting to apply the same configuration twice. I also removed the call to ast_sorcery_apply_config from res_mwi_external since it is no longer necessary either. 2) Adjust sorcery_apply_wizard_mapping() not to apply the same wizard to an object type more than once, just in case someone does make the error of calling ast_sorcery_apply_config() multiple times for the same object type. Diffs - /branches/12/tests/test_sorcery_realtime.c 410696 /branches/12/tests/test_sorcery_astdb.c 410696 /branches/12/tests/test_sorcery.c 410696 /branches/12/res/res_pjsip/pjsip_configuration.c 410696 /branches/12/res/res_pjsip/config_system.c 410696 /branches/12/res/res_mwi_external.c 410696 /branches/12/main/sorcery.c 410696 /branches/12/include/asterisk/sorcery.h 410696 /branches/12/configs/sorcery.conf.sample 410696 Diff: https://reviewboard.asterisk.org/r/3326/diff/ Testing --- My tests of retrieving data from realtime now get the expected objects. I don't have any automated tests to post yet because the realtime testsuite is a work in progress. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3353: testsuite: Test for receiving Play/Record start and stop events on ARI bridge playback/recording.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3353/ --- (Updated March 24, 2014, 5:36 p.m.) Review request for Asterisk Developers, Matt Jordan and Mark Michelson. Changes --- Removed the second application. All events are now expected on the testsuite application. Bugs: ASTERISK-23444 https://issues.asterisk.org/jira/browse/ASTERISK-23444 Repository: testsuite Description --- As mmichelson suggested, here are a couple automated tests for replicating the tests I did by hand in https://reviewboard.asterisk.org/r/3340/ The mechanism is fairly simple... Channel enters stasis and starts the whole process Mixing bridge gets created via ARI An application subscribes to bridge stasis messages for the newly created bridge Channel gets pushed into the bridge via ARI Bridge is recorded via ARI / Bridge playback with sound:tt-weasels is executed via ARI The sound comes naturally to an end / the recording is stopped manually upon receiving its startup event The test closes in the same way as a normal bridge subscription test Diffs (updated) - /asterisk/trunk/tests/rest_api/bridges/tests.yaml 4836 /asterisk/trunk/tests/rest_api/bridges/bridge_record/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_record/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_record/bridges_record.py PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/test-config.yaml PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/rest_api/bridges/bridge_play/bridges_play.py PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3353/diff/ Testing --- ran both tests some number of times to ensure success was repeatable. Varied some expectations to make sure everything I was looking for was actually being checked appropriately. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3326: Sorcery: Do not apply the same wizard to an object type twice; Automatically apply sorcery configuration when sorcery is opened.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3326/ --- (Updated March 24, 2014, 10:39 p.m.) Review request for Asterisk Developers. Changes --- Latest feedback has been addressed. Repository: Asterisk Description --- When performing some realtime tests, I noticed that the AMI command PJSIPShowEndpoints was listing all of my endpoints twice. This is because ast_sorcery_apply_config() was being called twice from res_pjsip code, once when initializing system configuration, and once again when initializing the rest of the configuration data. This patch aims to fix the problem on two fronts: 1) Remove the ast_sorcery_apply_config() calls from the PJSIP code entirely in favor of having sorcery automatically apply configuration for the module when sorcery is opened. This reduces the chance of accidentally attempting to apply the same configuration twice. I also removed the call to ast_sorcery_apply_config from res_mwi_external since it is no longer necessary either. 2) Adjust sorcery_apply_wizard_mapping() not to apply the same wizard to an object type more than once, just in case someone does make the error of calling ast_sorcery_apply_config() multiple times for the same object type. Diffs (updated) - /branches/12/tests/test_sorcery_realtime.c 411020 /branches/12/tests/test_sorcery_astdb.c 411020 /branches/12/tests/test_sorcery.c 411020 /branches/12/res/res_pjsip/pjsip_configuration.c 411020 /branches/12/res/res_pjsip/config_system.c 411020 /branches/12/res/res_mwi_external.c 411020 /branches/12/main/sorcery.c 411020 /branches/12/main/bucket.c 411020 /branches/12/include/asterisk/sorcery.h 411020 /branches/12/configs/sorcery.conf.sample 411020 Diff: https://reviewboard.asterisk.org/r/3326/diff/ Testing --- My tests of retrieving data from realtime now get the expected objects. I don't have any automated tests to post yet because the realtime testsuite is a work in progress. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3326: Sorcery: Do not apply the same wizard to an object type twice; Automatically apply sorcery configuration when sorcery is opened.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3326/#review11363 --- Ship it! I'll say this is ready to go now. :) - rmudgett On March 24, 2014, 5:39 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3326/ --- (Updated March 24, 2014, 5:39 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When performing some realtime tests, I noticed that the AMI command PJSIPShowEndpoints was listing all of my endpoints twice. This is because ast_sorcery_apply_config() was being called twice from res_pjsip code, once when initializing system configuration, and once again when initializing the rest of the configuration data. This patch aims to fix the problem on two fronts: 1) Remove the ast_sorcery_apply_config() calls from the PJSIP code entirely in favor of having sorcery automatically apply configuration for the module when sorcery is opened. This reduces the chance of accidentally attempting to apply the same configuration twice. I also removed the call to ast_sorcery_apply_config from res_mwi_external since it is no longer necessary either. 2) Adjust sorcery_apply_wizard_mapping() not to apply the same wizard to an object type more than once, just in case someone does make the error of calling ast_sorcery_apply_config() multiple times for the same object type. Diffs - /branches/12/tests/test_sorcery_realtime.c 411020 /branches/12/tests/test_sorcery_astdb.c 411020 /branches/12/tests/test_sorcery.c 411020 /branches/12/res/res_pjsip/pjsip_configuration.c 411020 /branches/12/res/res_pjsip/config_system.c 411020 /branches/12/res/res_mwi_external.c 411020 /branches/12/main/sorcery.c 411020 /branches/12/main/bucket.c 411020 /branches/12/include/asterisk/sorcery.h 411020 /branches/12/configs/sorcery.conf.sample 411020 Diff: https://reviewboard.asterisk.org/r/3326/diff/ Testing --- My tests of retrieving data from realtime now get the expected objects. I don't have any automated tests to post yet because the realtime testsuite is a work in progress. Thanks, Mark Michelson -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3374: testsuite: Verify that each of three users entering a conference enters muted when using the startmuted user profile option
On March 24, 2014, 4:39 p.m., Mark Michelson wrote: What are the files in the configs/ast2/ directory used for? Absolutely nothing. Removing. - Jonathan --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3374/#review11359 --- On March 18, 2014, 2:54 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3374/ --- (Updated March 18, 2014, 2:54 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23461 https://issues.asterisk.org/jira/browse/ASTERISK-23461 Repository: testsuite Description --- This relies on the addition of a test event in the conf_update_user_mute function: ast_test_suite_event_notify(CONF_MUTE_UPDATE, Mode: %s\r\n Conference: %s\r\n Channel: %s, mute_effective ? muted : unmuted, user-b_profile.name, ast_channel_name(user-chan)); The test is pretty simple. Three users enter a conference one after another while using a user profile with the startmuted option set. When all of the users have entered, the test is shut down. If any of those users' channels didn't receive the CONF_MUTE_UPDATE event or if the Mode for that event isn't muted, the test is failed. Diffs - /asterisk/trunk/tests/apps/confbridge/tests.yaml 4872 /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast2/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/manager.general.conf.inc PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/confbridge.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3374/diff/ Testing --- Ran the test, confirmed the events, set the expectations of some things to intentional failures to verify that it would fail if mismatches occur. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3374: testsuite: Verify that each of three users entering a conference enters muted when using the startmuted user profile option
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3374/ --- (Updated March 24, 2014, 5:54 p.m.) Review request for Asterisk Developers. Changes --- shuffling some events. Ditched vestigial configuration files. Bugs: ASTERISK-23461 https://issues.asterisk.org/jira/browse/ASTERISK-23461 Repository: testsuite Description --- This relies on the addition of a test event in the conf_update_user_mute function: ast_test_suite_event_notify(CONF_MUTE_UPDATE, Mode: %s\r\n Conference: %s\r\n Channel: %s, mute_effective ? muted : unmuted, user-b_profile.name, ast_channel_name(user-chan)); The test is pretty simple. Three users enter a conference one after another while using a user profile with the startmuted option set. When all of the users have entered, the test is shut down. If any of those users' channels didn't receive the CONF_MUTE_UPDATE event or if the Mode for that event isn't muted, the test is failed. Diffs (updated) - /asterisk/trunk/tests/apps/confbridge/tests.yaml 4872 /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/test-config.yaml PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/manager.general.conf.inc PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/apps/confbridge/confbridge_start_muted/configs/ast1/confbridge.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3374/diff/ Testing --- Ran the test, confirmed the events, set the expectations of some things to intentional failures to verify that it would fail if mismatches occur. 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] 3386: Dialplan functions: Fix NULL channel safety issues
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3386/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23391 https://issues.asterisk.org/jira/browse/ASTERISK-23391 Repository: Asterisk Description --- This is the Asterisk 12 version of fixes for dialplan functions handling of NULL channel. The patch for trunk is identical to 12. Patches for 1.8 and 11 are smaller, those patches are posted to JIRA. I can post separate reviews for 1.8 and 11 if anyone asks. Looking to the future, I think it would be better to add a 3rd bit-field to 'struct ast_custom_function' to allow_global, have pbx.c check if channel is NULL. This would be cleaner with functions using RAII or SCOPED_CHANNELLOCK (main/features_config.c), and remove the check from a large number of functions. Diffs - /branches/12/res/res_xmpp.c 410669 /branches/12/res/res_pjsip_header_funcs.c 410669 /branches/12/res/res_mutestream.c 410669 /branches/12/res/res_jabber.c 410669 /branches/12/res/res_calendar.c 410669 /branches/12/main/message.c 410669 /branches/12/main/features_config.c 410669 /branches/12/funcs/func_volume.c 410669 /branches/12/funcs/func_strings.c 410669 /branches/12/funcs/func_speex.c 410669 /branches/12/funcs/func_pitchshift.c 410669 /branches/12/funcs/func_odbc.c 410669 /branches/12/funcs/func_math.c 410669 /branches/12/funcs/func_jitterbuffer.c 410669 /branches/12/funcs/func_groupcount.c 410669 /branches/12/funcs/func_global.c 410669 /branches/12/funcs/func_frame_trace.c 410669 /branches/12/funcs/func_dialplan.c 410669 /branches/12/funcs/func_channel.c 410669 /branches/12/funcs/func_cdr.c 410669 /branches/12/funcs/func_callerid.c 410669 /branches/12/funcs/func_callcompletion.c 410669 /branches/12/funcs/func_blacklist.c 410669 /branches/12/channels/pjsip/dialplan_functions.c 410669 /branches/12/channels/chan_sip.c 410669 /branches/12/channels/chan_iax2.c 410669 /branches/12/apps/confbridge/conf_config_parser.c 410669 /branches/12/apps/app_voicemail.c 410669 /branches/12/apps/app_stack.c 410669 /branches/12/apps/app_speech_utils.c 410669 /branches/12/apps/app_jack.c 410669 Diff: https://reviewboard.asterisk.org/r/3386/diff/ Testing --- Compiled, visually inspected. I cannot compile app_jack due to dependencies, all other changed files compiled with devmode. 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
