Re: [asterisk-dev] [Code Review] 3550: build: Allow autoconf/ast_ext_tool_check to handle cross-compiling better
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3550/
---
(Updated May 20, 2014, 3 p.m.)
Review request for Asterisk Developers.
Changes
---
Oops...Used bash-specific pattern substitution. Converted to sed and retested.
Repository: Asterisk
Description
---
ast_ext_tool_check.m4 isn't handling cases where a path to a package is
provided (E.G. --with-mysqlclient=/some/sysroot) and the package has a config
tool (E.G. mysql_config) and the package has its own subdirectories in include
or lib. For example, mysql's libraries are in ${MYSQLCLIENT_DIR}/usr/lib/mysql
but ast_ext_tool_check sets MYSQLCLIENT_LIB to ${MYSQLCLIENT_DIR}/usr/lib.
libxml2 has the same problem with its includes. They're in
${LIBXML2_DIR}/usr/include/libxml2 not directly in ${LIBXML2_DIR}/usr/include.
Both cause configure to fail and there are others in the same boat.
The problem is caused by logic in ast_ext_tool_check that overrides the result
of the config tool's --cflags and --libs options if package_DIR is set.
This patch prepends package_DIR (if specified) to the -L and -I results from
the package's config tool instead of overriding them.
I also did a little reformatting. It was ugly.
Diffs (updated)
-
branches/11/autoconf/ast_ext_tool_check.m4 414212
Diff: https://reviewboard.asterisk.org/r/3550/diff/
Testing
---
Tested with cross compile for armv7hl platform (package paths specified) and
with native x86_64 compile (no package paths specified).
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. Diffs - branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2.c 414877 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
On May 30, 2014, 2:38 p.m., rmudgett wrote: branches/12/main/astobj2_private.h, lines 156-159 https://reviewboard.asterisk.org/r/3576/diff/1/?file=59035#file59035line156 There is too much in this file that should not be here. 1) Almost everything from this enum definition and above should stay where it was in astobj2. It does not need to be spread outside of that file. 2) The INTERNAL_OBJ() references in the container code is easily abstracted to an internal get the ao2_options value API call. 3) The hash container specific declarations in this file should be only in the astobj2_hash.c file. The same should be for the rbtree declarations respectively. 4) About the only stuff remaining in this file should be some miscellaneous function prototypes and generic container and node definitions common to all containers. ao2 containers are built on top of ao2 objects. I laid out the astobj2.c file into sections which for the most part could be easily split into their own files. 1) astobj2 object primitives code 2) astobj2 container generic code 3) astobj2 container hash specific code 4) astobj2 container rbtree specific code 5) astobj2 miscellaneous code (unit test, CLI debug commands, initialization) Reference https://reviewboard.asterisk.org/r/2110/ for mmichelson's comment about breaking astobj2.c up. I've covered most of the contents of the exchange here. The reason I moved the hash-specific stuff into the common header file is that the weak-reference implementation re-uses a good chunk of the hash implementation...all of the structures and most of the code. The rbtree stuff was moved just to be consistent. Knowing that, what would you like me to do? Do you want me to further break out the container common code into astobj2_container.c and the misc code into astobj2_misc.c? - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/#review12014 --- On May 29, 2014, 6:53 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated May 29, 2014, 6:53 p.m.) Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. Diffs - branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2.c 414877 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated May 30, 2014, 11:52 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Updated with rmudgett's direction. astobj2.c - object primitives, object primitive misc and initialization code. astobj2_private.h - internal object primitive declarations needed by the containers. astobj2_container.c - generic conainer and container misc code astobj2_container_hash.c - hash container specific code astobj2_container_rbtree.c - rbtree container specific code astobj2_container_private.h - generic container definitions and rtti prototypes Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. Diffs (updated) - branches/12/tests/test_astobj2.c 414969 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 414969 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated May 30, 2014, 11:54 p.m.) Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description (updated) --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs - branches/12/tests/test_astobj2.c 414969 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 414969 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated June 4, 2014, 7:33 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Addressed Richard's comments. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs (updated) - branches/12/utils/Makefile 415187 branches/12/tests/test_astobj2.c 415187 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 415187 branches/12/include/asterisk/astobj2.h 415187 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
On June 4, 2014, 3:47 p.m., rmudgett wrote: branches/12/tests/test_astobj2.c, lines 2002-2003 https://reviewboard.asterisk.org/r/3576/diff/2/?file=59061#file59061line2002 Compiler error: ast_tvdiff_ms(ast_tvnow(), start) does not match the format string. ast_tvdiff_ms() returns int64_t. Probably best to cast it to (unsigned long). No error for me but I added the cast. On June 4, 2014, 3:47 p.m., rmudgett wrote: branches/12/main/astobj2.c, line 30 https://reviewboard.asterisk.org/r/3576/diff/2/?file=59055#file59055line30 Drat! Splitting astobj2 makes the utils directory no longer compile. That's going to suck a bit getting those hacked up utilities to compile again. Just had to update the utils/Makefile to include copying the new files. Also to get refcounter to compile without complaining, I had to remove the force of no optimization. This problem has existed for me for a while. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/#review12048 --- On June 4, 2014, 7:33 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated June 4, 2014, 7:33 p.m.) Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs - branches/12/utils/Makefile 415187 branches/12/tests/test_astobj2.c 415187 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 415187 branches/12/include/asterisk/astobj2.h 415187 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated June 5, 2014, 4:38 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Addressed Richard's issues. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs (updated) - branches/12/utils/Makefile 415300 branches/12/tests/test_astobj2.c 415300 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 415300 branches/12/include/asterisk/astobj2.h 415300 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
On June 5, 2014, 4:02 p.m., rmudgett wrote: Also the utils directory svn:ignore property needs to be updated to include the new astobj2 files. Done. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/#review12067 --- On June 5, 2014, 4:38 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated June 5, 2014, 4:38 p.m.) Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs - branches/12/utils/Makefile 415300 branches/12/tests/test_astobj2.c 415300 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 415300 branches/12/include/asterisk/astobj2.h 415300 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated June 5, 2014, 5:43 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Addressed more of Richard's issues. Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs (updated) - branches/12/utils/Makefile 415300 branches/12/tests/test_astobj2.c 415300 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 415300 branches/12/include/asterisk/astobj2.h 415300 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3576: astobj2: Split hash and rbtree impls into their own source files.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3576/ --- (Updated June 6, 2014, 9:08 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and rmudgett. Changes --- Committed in revision 415317 Repository: Asterisk Description --- In preparation for weak-reference containers, and because it makes the existing code easier to read and maintain, I've split the astobj2 common structure and enum definitions and prototypes into astobj2_private.h, the hash table implementation into astobj2_hash.c, and the rbtree implementation into astobj2_rbtree.c. All of the public functions remain in astobj2.c. A few functions (adjust_lock, container_destruct, container_destruct_debug) needed to have their static modifiers removed so they'd be visible from the other object files but other than that there were NO functional changes, no logic changes, etc. EDIT:.. Also added a basic test to the test framework to monitor performance impacts as changes are made to astobj2. Diffs - branches/12/utils/Makefile 415300 branches/12/tests/test_astobj2.c 415300 branches/12/main/astobj2_rbtree.c PRE-CREATION branches/12/main/astobj2_private.h PRE-CREATION branches/12/main/astobj2_hash.c PRE-CREATION branches/12/main/astobj2_container_private.h PRE-CREATION branches/12/main/astobj2_container.c PRE-CREATION branches/12/main/astobj2.c 415300 branches/12/include/asterisk/astobj2.h 415300 Diff: https://reviewboard.asterisk.org/r/3576/diff/ Testing --- I used both the test framework and the test suite. For the test suite, I used channels/pjsip since that exercises sorcery significantly and that in turn exercises astobj2. All tests that worked before the change worked after the change. Before... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint After... Test Framework 393 Test(s) Executed 393 Passed 0 Failed Test Suite tests/channels/pjsip/ Tests: 88 Passed: 87 Failed: 1 FAILED: tests/channels/pjsip/dialplan_functions/pjsip_endpoint Not sure why the pjsip_endpoint function is failing but it's not this patch's fault. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs - branches/12/main/astobj2_rbtree.c 415336 branches/12/main/astobj2_hash.c 415336 branches/12/main/astobj2_container_private.h 415336 branches/12/main/astobj2_container.c 415336 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 6, 2014, 1:49 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Addressed Richard's comments concerning sort_fn being a prereq for a container integrity check. However... Removing the sort_fn check is highlighting issues, mostly in main/format, that are causing lots of integrity check failed error messages to be printed to the console. The issues are pre-existing and all test framework tests pass, it's just that there are now errors being printed for them. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs (updated) - branches/12/main/astobj2_rbtree.c 415336 branches/12/main/astobj2_hash.c 415336 branches/12/main/astobj2_container_private.h 415336 branches/12/main/astobj2_container.c 415336 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Astobj2 debugging change proposal
Right now, the non-ref debugging code in astobj2 is triggered by a mix of AST_DEVMODE and AO2_DEBUG and both get set if you want to run the test framework. I've noticed though that the inclusion of the debugging code can actually hide problems as well as highlight them, especially related to performance and locking. Case in point: Try running 'test execute category /main/astobj2 name thrash' with REF_DEBUG turned on and off. While the test passes both ways, the timing is significantly different (and not what you'd expect). Luckily, you can turn REF_DEBUG on and off from menuselect. To provide a little more flexibility and visibility, I'd like to propose the following... 1. Change astobj2 so the non-ref debugging code is dependent on AO2_DEBUG solely instead of the mix of AST_DEVMODE and AO2_DEBUG. (REF_DEBUG would remain as is) 2. Remove the code that automatically sets AO2_DEBUG if TEST_FRAMEWORK is defined. 3. Add AO2_DEBUG to the menuselect extended compiler flags right next to REF_DEBUG. This way you can run the test framework in either production or debugging mode. The code change to do this is trivial but I thought I'd run it by you guys first. Thoughts? -- _ -- 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] Astobj2 debugging change proposal
On Mon, Jun 9, 2014 at 11:22 AM, Corey Farrell g...@cfware.com wrote: One thing I dislike about AST_DEVMODE is that changing it requires rerunning ./configure. I feel ./configure should be restricted as much as possible to checking dependencies. Any build configuration that is not directly enabled/disabled by an external dependency should be in menuselect. If we are going to make it so TEST_FRAMEWORK doesn't enable AO2_DEBUG, I think we should allow Bamboo to run without AO2_DEBUG once. This way if any tests actually require it we can update dependencies. I ran the testsuite last night without AO2_DEBUG and didn't notice anything different but I'm going to run it again now. I'm not sure how I feel about AO2_DEBUG being combined with REF_DEBUG. AO2_DEBUG doesn't appear to add much overhead, but REF_DEBUG adds significant overhead through output to the refs log. I'm not sure someone who specifically wants AO2_DEBUG would want REF_DEBUG. Especially since REF_DEBUG can change timing and that could change results. Agreed. REF_DEBUG seems more aimed at debugging the use of astobj2 whereas AO2_DEBUG is more aimed at debugging astobj2 itself. 2 different purposes. On Mon, Jun 9, 2014 at 12:46 PM, Matthew Jordan mjor...@digium.com wrote: On Sun, Jun 8, 2014 at 10:03 AM, George Joseph george.jos...@fairview5.com wrote: Right now, the non-ref debugging code in astobj2 is triggered by a mix of AST_DEVMODE and AO2_DEBUG and both get set if you want to run the test framework. I've noticed though that the inclusion of the debugging code can actually hide problems as well as highlight them, especially related to performance and locking. Case in point: Try running 'test execute category /main/astobj2 name thrash' with REF_DEBUG turned on and off. While the test passes both ways, the timing is significantly different (and not what you'd expect). Luckily, you can turn REF_DEBUG on and off from menuselect. To provide a little more flexibility and visibility, I'd like to propose the following... 1. Change astobj2 so the non-ref debugging code is dependent on AO2_DEBUG solely instead of the mix of AST_DEVMODE and AO2_DEBUG. (REF_DEBUG would remain as is) Having two different 'dev' or 'debug' compilation flags is confusing. I'd be happy with having them collapsed down to one. 2. Remove the code that automatically sets AO2_DEBUG if TEST_FRAMEWORK is defined. Richard may want to chime in on this, since I'm pretty sure that was a change he initiated. Personally, I think it is okay if they are decoupled. We can always enable AO2_DEBUG from menuselect in Bamboo build plans, and I'm not aware of any Asterisk Test Suite functionality that explicitly relies on AO2_DEBUG being defined. 3. Add AO2_DEBUG to the menuselect extended compiler flags right next to REF_DEBUG. I'm okay with this too... although is there a reason why REF_DEBUG and AO2_DEBUG shouldn't be combined? Do they absolutely have to be separate compilation flags? This way you can run the test framework in either production or debugging mode. The code change to do this is trivial but I thought I'd run it by you guys first. Thoughts? -- Matthew Jordan Digium, Inc. | Engineering Manager 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com http://asterisk.org -- _ -- 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 -- _ -- 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 -- _ -- 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] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 10, 2014, 8:42 a.m.) Review request for Asterisk Developers and rmudgett. Changes --- Updated patch to include the AO2_DEBUG changes discussed on the asterisk-dev list. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs (updated) - branches/12/tests/test_astobj2.c 415657 branches/12/main/astobj2_rbtree.c 415657 branches/12/main/astobj2_private.h 415657 branches/12/main/astobj2_hash.c 415657 branches/12/main/astobj2_container_private.h 415657 branches/12/main/astobj2_container.c 415657 branches/12/build_tools/cflags.xml 415657 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] chan_sip usereqphone option
On Wed, Jun 11, 2014 at 12:24 PM, Mark Michelson mmichel...@digium.com wrote: Hi! It has been brought to my attention that chan_pjsip does not have an equivalent to chan_sip's usereqphone option. However, nobody here seems to know how useful such an option actually would be. The result is, we have no idea if an equivalent should be made in chan_pjsip or if we should just leave that option alone. I figure that the -dev list may have some experience with setting up accounts with a variety of SIP providers and could shed some light on the usefulness of that option. Thanks. I've set up a dozen providers but never needed usereqphone. For pjsip though couldn't you just add ';user=phone' to a contact uri if you needed it? -- _ -- 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] 3627: Update extensions.lua.sample with naming conflict guidance.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3627/ --- Review request for Asterisk Developers and rnewton. Bugs: asterisk-23844 https://issues.asterisk.org/jira/browse/asterisk-23844 Repository: Asterisk Description --- The sample extensions.lua was causing pbx_lua to fail to load when parsing 'app.goto(default, s, 1)' because in Lua 5.2, 'goto' is now a reserved word. Added the following guidance to extensions.lua.sample and changed 'app.goto(default, s, 1)' to 'app.['goto'](default, s, 1)'. -- Note about naming conflicts: -- Lua allows you to refer to table entries using the '.' notation, -- I.E. app.goto(something), only if the entry doesn't conflict with an Lua -- reserved word. In the 'goto' example, with Lua 5.1 or earlier, 'goto' is -- not a reserved word so you'd be calling the Asterisk dialplan application -- 'goto'. Lua 5.2 however, introduced the 'goto' control statement which -- makes 'goto' a reserved word. This casues the interpreter to fail parsing -- the file and pbx_lua.so will fail to load. The same applies to any use of -- Lua tables including extensions, channels and any tables you create. -- -- There are two ways around this: Since Lua is case-sensitive, you can use -- capitalized names, I.E. app.Goto(something) to refer to the Asterisk apps, -- functions, etc. Or you can use the full syntax, I.E. app[goto](something). -- Both syntaxes are backwards compatible with earlier Lua versions. To make -- your Lua dialplans easier to maintain and to reduce the chance of future -- conflicts you may want to use the app[goto](something) syntax for all -- table accesses. -- This patch will merge through to 11, 12 and trunk. The wiki needs to be update with the same info. Diffs - branches/1.8/configs/extensions.lua.sample 416556 Diff: https://reviewboard.asterisk.org/r/3627/diff/ Testing --- Made sure extensions.lua now loads correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3627: Update extensions.lua.sample with naming conflict guidance.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3627/ --- (Updated June 18, 2014, 11:41 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and rnewton. Changes --- Committed in revision 416578 Bugs: asterisk-23844 https://issues.asterisk.org/jira/browse/asterisk-23844 Repository: Asterisk Description --- The sample extensions.lua was causing pbx_lua to fail to load when parsing 'app.goto(default, s, 1)' because in Lua 5.2, 'goto' is now a reserved word. Added the following guidance to extensions.lua.sample and changed 'app.goto(default, s, 1)' to 'app.['goto'](default, s, 1)'. -- Note about naming conflicts: -- Lua allows you to refer to table entries using the '.' notation, -- I.E. app.goto(something), only if the entry doesn't conflict with an Lua -- reserved word. In the 'goto' example, with Lua 5.1 or earlier, 'goto' is -- not a reserved word so you'd be calling the Asterisk dialplan application -- 'goto'. Lua 5.2 however, introduced the 'goto' control statement which -- makes 'goto' a reserved word. This casues the interpreter to fail parsing -- the file and pbx_lua.so will fail to load. The same applies to any use of -- Lua tables including extensions, channels and any tables you create. -- -- There are two ways around this: Since Lua is case-sensitive, you can use -- capitalized names, I.E. app.Goto(something) to refer to the Asterisk apps, -- functions, etc. Or you can use the full syntax, I.E. app[goto](something). -- Both syntaxes are backwards compatible with earlier Lua versions. To make -- your Lua dialplans easier to maintain and to reduce the chance of future -- conflicts you may want to use the app[goto](something) syntax for all -- table accesses. -- This patch will merge through to 11, 12 and trunk. The wiki needs to be update with the same info. Diffs - branches/1.8/configs/extensions.lua.sample 416556 Diff: https://reviewboard.asterisk.org/r/3627/diff/ Testing --- Made sure extensions.lua now loads correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3629: pbx_lua: Remove the problematic and unnecessary AST_MODFLAG_GLOBAL_SYMBOLS from pbx_lua.c
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3629/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23818 https://issues.asterisk.org/jira/browse/ASTERISK-23818 Repository: Asterisk Description --- AST_MODFLAG_GLOBAL_SYMBOLS was causing the module to be incorrectly loaded before pbx_config. pbx_config was therefore blowing away contexts that were created by pbx_lua. With AST_MODFLAG_DEFAULT the load order is now correct and contexs are being properly merged. AST_MODFLAG_GLOBAL_SYMBOLS was not needed anyway since no other modules needed its global symbols that early. This patch needs to be merged all the way though trunk. Diffs - branches/1.8/pbx/pbx_lua.c 416661 Diff: https://reviewboard.asterisk.org/r/3629/diff/ Testing --- Checked that contexts created by both pbx_config and pbx_lua were properly merged instead of pbx_config overwriting them. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3629: pbx_lua: Remove the problematic and unnecessary AST_MODFLAG_GLOBAL_SYMBOLS from pbx_lua.c
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3629/ --- (Updated June 19, 2014, 10:59 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 416667 Bugs: ASTERISK-23818 https://issues.asterisk.org/jira/browse/ASTERISK-23818 Repository: Asterisk Description --- AST_MODFLAG_GLOBAL_SYMBOLS was causing the module to be incorrectly loaded before pbx_config. pbx_config was therefore blowing away contexts that were created by pbx_lua. With AST_MODFLAG_DEFAULT the load order is now correct and contexs are being properly merged. AST_MODFLAG_GLOBAL_SYMBOLS was not needed anyway since no other modules needed its global symbols that early. This patch needs to be merged all the way though trunk. Diffs - branches/1.8/pbx/pbx_lua.c 416661 Diff: https://reviewboard.asterisk.org/r/3629/diff/ Testing --- Checked that contexts created by both pbx_config and pbx_lua were properly merged instead of pbx_config overwriting them. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3652: pjsip cli: Change Identify to show CIDR notation instead of netmasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3652/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- In 'pjsip show endpoints' wouldn't you rather see... Identify: 192.168.101.54/32,192.168.147.1/32 or Identify: 26ff:ff:6622:e020::220/128 instead of... Identify: 192.168.101.54/255.255.255.255,192.168.147.1/255.255.255.255 or Identify: 26ff:ff:6622:e020::220/::::::: ? I would. :) Added ast_sockaddr_cidr_bits() to count the 1 bits in an ast_sockaddr. Added ast_ha_join_cidr() which uses ast_sockaddr_cidr_bits() for the netmask instead of ast_sockaddr_stringify_addr. Changed res_pjsip_endpoint_identifier_ip to call ast_ha_join_cidr() instead of ast_ha_join(). This is a CLI change only. AMI was not affected. Diffs - branches/12/res/res_pjsip_endpoint_identifier_ip.c 416729 branches/12/main/netsock2.c 416729 branches/12/main/acl.c 416729 branches/12/include/asterisk/netsock2.h 416729 branches/12/include/asterisk/acl.h 416729 Diff: https://reviewboard.asterisk.org/r/3652/diff/ Testing --- Made sure both ipv4 and ipv6 addresses were formatted correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3652: pjsip cli: Change Identify to show CIDR notation instead of netmasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3652/ --- (Updated June 19, 2014, 1:29 p.m.) Review request for Asterisk Developers. Changes --- Addressed Mark's comments. Repository: Asterisk Description --- In 'pjsip show endpoints' wouldn't you rather see... Identify: 192.168.101.54/32,192.168.147.1/32 or Identify: 26ff:ff:6622:e020::220/128 instead of... Identify: 192.168.101.54/255.255.255.255,192.168.147.1/255.255.255.255 or Identify: 26ff:ff:6622:e020::220/::::::: ? I would. :) Added ast_sockaddr_cidr_bits() to count the 1 bits in an ast_sockaddr. Added ast_ha_join_cidr() which uses ast_sockaddr_cidr_bits() for the netmask instead of ast_sockaddr_stringify_addr. Changed res_pjsip_endpoint_identifier_ip to call ast_ha_join_cidr() instead of ast_ha_join(). This is a CLI change only. AMI was not affected. Diffs (updated) - branches/12/res/res_pjsip_endpoint_identifier_ip.c 416729 branches/12/main/netsock2.c 416729 branches/12/main/acl.c 416729 branches/12/include/asterisk/netsock2.h 416729 branches/12/include/asterisk/acl.h 416729 Diff: https://reviewboard.asterisk.org/r/3652/diff/ Testing --- Made sure both ipv4 and ipv6 addresses were formatted correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3652: pjsip cli: Change Identify to show CIDR notation instead of netmasks.
On June 19, 2014, 12:01 p.m., Mark Michelson wrote:
branches/12/main/netsock2.c, lines 156-160
https://reviewboard.asterisk.org/r/3652/diff/1/?file=59870#file59870line156
This loop is incorrect. I believe it should be:
if (j = 0; j 8; ++j) {
if ((addr[i] j) 1) {
bits++;
}
}
Mark Michelson wrote:
Obviously that first line should be for not if :)
I swear I typed j instead of 1 in that if. :)
On June 19, 2014, 12:01 p.m., Mark Michelson wrote:
branches/12/main/acl.c, line 683
https://reviewboard.asterisk.org/r/3652/diff/1/?file=59869#file59869line683
It's typically a good idea to avoid ast_strdupa() inside a for loop
since it theoretically could blow the stack on a very large list.
Since addr is const, you don't even need to make a stack copy of it at
all.
This was a cut and paste from the original ast_ha_join. I removed the strdup
here and there don't seem to be any ill effects but I left ast_ha_join alone as
it would require more testing. After all, somebody put it there for a reason.
:)
- George
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3652/#review12209
---
On June 19, 2014, 11:47 a.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3652/
---
(Updated June 19, 2014, 11:47 a.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
In 'pjsip show endpoints' wouldn't you rather see...
Identify: 192.168.101.54/32,192.168.147.1/32
or
Identify: 26ff:ff:6622:e020::220/128
instead of...
Identify: 192.168.101.54/255.255.255.255,192.168.147.1/255.255.255.255
or
Identify: 26ff:ff:6622:e020::220/:::::::
?
I would. :)
Added ast_sockaddr_cidr_bits() to count the 1 bits in an ast_sockaddr.
Added ast_ha_join_cidr() which uses ast_sockaddr_cidr_bits() for the netmask
instead of ast_sockaddr_stringify_addr.
Changed res_pjsip_endpoint_identifier_ip to call ast_ha_join_cidr() instead
of ast_ha_join().
This is a CLI change only. AMI was not affected.
Diffs
-
branches/12/res/res_pjsip_endpoint_identifier_ip.c 416729
branches/12/main/netsock2.c 416729
branches/12/main/acl.c 416729
branches/12/include/asterisk/netsock2.h 416729
branches/12/include/asterisk/acl.h 416729
Diff: https://reviewboard.asterisk.org/r/3652/diff/
Testing
---
Made sure both ipv4 and ipv6 addresses were formatted correctly.
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3652: pjsip cli: Change Identify to show CIDR notation instead of netmasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3652/ --- (Updated June 19, 2014, 3:12 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 416737 Repository: Asterisk Description --- In 'pjsip show endpoints' wouldn't you rather see... Identify: 192.168.101.54/32,192.168.147.1/32 or Identify: 26ff:ff:6622:e020::220/128 instead of... Identify: 192.168.101.54/255.255.255.255,192.168.147.1/255.255.255.255 or Identify: 26ff:ff:6622:e020::220/::::::: ? I would. :) Added ast_sockaddr_cidr_bits() to count the 1 bits in an ast_sockaddr. Added ast_ha_join_cidr() which uses ast_sockaddr_cidr_bits() for the netmask instead of ast_sockaddr_stringify_addr. Changed res_pjsip_endpoint_identifier_ip to call ast_ha_join_cidr() instead of ast_ha_join(). This is a CLI change only. AMI was not affected. Diffs - branches/12/res/res_pjsip_endpoint_identifier_ip.c 416729 branches/12/main/netsock2.c 416729 branches/12/main/acl.c 416729 branches/12/include/asterisk/netsock2.h 416729 branches/12/include/asterisk/acl.h 416729 Diff: https://reviewboard.asterisk.org/r/3652/diff/ Testing --- Made sure both ipv4 and ipv6 addresses were formatted correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
On June 19, 2014, 1:31 p.m., rmudgett wrote: branches/12/main/astobj2_rbtree.c, lines 1324-1330 https://reviewboard.asterisk.org/r/3593/diff/3/?file=59520#file59520line1324 This is common code to any astobj2 container and should be pulled back up to the general container code in the AO2_CONTAINER_INSERT_NODE_INSERTED case. The goto inserted changes in this function could be reverted as a result. The hash container specific hash_ao2_link_node_stat() call that is done in the corresponding location of hash_ao2_insert_node() would still be done there. I pushed some of this stuff down for 2 reasons... 1. I think it's the container implementation's responsibility to make sure it's own stats are updated and the container is consistent, even though some of the code might be duplicated among container implementations. 2. It makes the common code harder to read and follow. Having said that... :) I pulled them back up. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/#review12206 --- On June 10, 2014, 8:42 a.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 10, 2014, 8:42 a.m.) Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs - branches/12/tests/test_astobj2.c 415657 branches/12/main/astobj2_rbtree.c 415657 branches/12/main/astobj2_private.h 415657 branches/12/main/astobj2_hash.c 415657 branches/12/main/astobj2_container_private.h 415657 branches/12/main/astobj2_container.c 415657 branches/12/build_tools/cflags.xml 415657 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 19, 2014, 3:26 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Addressed Richard's comments. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs (updated) - branches/12/tests/test_astobj2.c 416802 branches/12/main/astobj2_rbtree.c 416802 branches/12/main/astobj2_private.h 416802 branches/12/main/astobj2_hash.c 416802 branches/12/main/astobj2_container_private.h 416802 branches/12/main/astobj2_container.c 416802 branches/12/build_tools/cflags.xml 416802 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 19, 2014, 5:04 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Continuing to address Richard's comments. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs (updated) - branches/12/tests/test_astobj2.c 416802 branches/12/main/astobj2_rbtree.c 416802 branches/12/main/astobj2_private.h 416802 branches/12/main/astobj2_hash.c 416802 branches/12/main/astobj2_container_private.h 416802 branches/12/main/astobj2_container.c 416802 branches/12/build_tools/cflags.xml 416802 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 19, 2014, 5:47 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Continuing to address comments. Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs (updated) - branches/12/tests/test_astobj2.c 416802 branches/12/main/astobj2_rbtree.c 416802 branches/12/main/astobj2_private.h 416802 branches/12/main/astobj2_hash.c 416802 branches/12/main/astobj2_container_private.h 416802 branches/12/main/astobj2_container.c 416802 branches/12/build_tools/cflags.xml 416802 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3593: astobj2: Additional refactoring to push impl specific code down into the impls.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3593/ --- (Updated June 20, 2014, 10:22 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and rmudgett. Changes --- Committed in revision 416806 Repository: Asterisk Description --- Move some implementation specific code from astobj2_container.c into astobj2_hash.c and astobj2_rbtree.c. This completely removes the need for astobj2_container to switch on RTTI and it no longer has any knowledge of the implementation details. The RTTI itself is no longer needed but I don't want to remove it until weak reference containers are added in case I need it back again. If not, I'll remove it then. Diffs - branches/12/tests/test_astobj2.c 416802 branches/12/main/astobj2_rbtree.c 416802 branches/12/main/astobj2_private.h 416802 branches/12/main/astobj2_hash.c 416802 branches/12/main/astobj2_container_private.h 416802 branches/12/main/astobj2_container.c 416802 branches/12/build_tools/cflags.xml 416802 Diff: https://reviewboard.asterisk.org/r/3593/diff/ Testing --- Ran all the test framework tests plus the astobj2 test test. Ran testsuite tests/channels/pjsip. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3550: build: Allow autoconf/ast_ext_tool_check to handle cross-compiling better
On May 22, 2014, 8:36 a.m., Matt Jordan wrote:
I applied this to 1.8 and didn't run into any problems.
Can you make sure this doesn't run into any problems with pjproject in 12?
Oops...lost this one for a bit.
Works fine with pjproject in 12/trunk.
- George
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3550/#review11954
---
On May 20, 2014, 3 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3550/
---
(Updated May 20, 2014, 3 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
ast_ext_tool_check.m4 isn't handling cases where a path to a package is
provided (E.G. --with-mysqlclient=/some/sysroot) and the package has a config
tool (E.G. mysql_config) and the package has its own subdirectories in
include or lib. For example, mysql's libraries are in
${MYSQLCLIENT_DIR}/usr/lib/mysql but ast_ext_tool_check sets MYSQLCLIENT_LIB
to ${MYSQLCLIENT_DIR}/usr/lib. libxml2 has the same problem with its
includes. They're in ${LIBXML2_DIR}/usr/include/libxml2 not directly in
${LIBXML2_DIR}/usr/include. Both cause configure to fail and there are
others in the same boat.
The problem is caused by logic in ast_ext_tool_check that overrides the
result of the config tool's --cflags and --libs options if package_DIR is set.
This patch prepends package_DIR (if specified) to the -L and -I results from
the package's config tool instead of overriding them.
I also did a little reformatting. It was ugly.
Diffs
-
branches/11/autoconf/ast_ext_tool_check.m4 414212
Diff: https://reviewboard.asterisk.org/r/3550/diff/
Testing
---
Tested with cross compile for armv7hl platform (package paths specified) and
with native x86_64 compile (no package paths specified).
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3513: Weak Reference Containers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3513/ --- (Updated June 20, 2014, 1:42 p.m.) Status -- This change has been discarded. Review request for Asterisk Developers, Joshua Colp and rmudgett. Repository: Asterisk Description --- Add weak reference capability to hash and list astobj2 containers. Current (Strong-reference) 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-reference behavior: A container (list or hash) 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. The object's ref count can therefore validly reach 0 while still in a weak-ref container in which case the object is automatically and cleanly removed from any weak-ref container it may be in. Because of the complexity of the red-black tree implementation, it isn't eligible for weak-reference behavior. 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 reference to the instance so it doesn't get destroyed without special code to check the ref count on each call to close. The same thing happens with the pjsip cli command registry. 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. The test framework now has a /main/astobj2/perf/ category to show relative performance. NOTE: Previously, when the test framework was enabled AO2_DEBUG was also enabled but this was skewing the results so I removed that auto-enablement of AO2_DEBUG. The test framework never needed it. The original RFC review request was here... https://reviewboard.asterisk.org/r/3382/ The major changes from the RFC are that a spinlock was added around the object's ref-counter operations and the rbtree's are now excluded from being weak-reference containers. Diffs - branches/12/tests/test_astobj2_thrash.c 413157 branches/12/tests/test_astobj2.c 413157 branches/12/main/astobj2.c 413157 branches/12/include/asterisk/astobj2.h 413157 Diff: https://reviewboard.asterisk.org/r/3513/diff/ Testing --- 7 new tests were added to the existing astobj2 test framework. All tests pass. The testsuite is a little more challenging but I at least made sure that all the test that passed before the change still passed after the change. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3550: build: Allow autoconf/ast_ext_tool_check to handle cross-compiling better
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3550/
---
(Updated June 20, 2014, 4:55 p.m.)
Status
--
This change has been marked as submitted.
Review request for Asterisk Developers.
Changes
---
Committed in revision 416869
Repository: Asterisk
Description
---
ast_ext_tool_check.m4 isn't handling cases where a path to a package is
provided (E.G. --with-mysqlclient=/some/sysroot) and the package has a config
tool (E.G. mysql_config) and the package has its own subdirectories in include
or lib. For example, mysql's libraries are in ${MYSQLCLIENT_DIR}/usr/lib/mysql
but ast_ext_tool_check sets MYSQLCLIENT_LIB to ${MYSQLCLIENT_DIR}/usr/lib.
libxml2 has the same problem with its includes. They're in
${LIBXML2_DIR}/usr/include/libxml2 not directly in ${LIBXML2_DIR}/usr/include.
Both cause configure to fail and there are others in the same boat.
The problem is caused by logic in ast_ext_tool_check that overrides the result
of the config tool's --cflags and --libs options if package_DIR is set.
This patch prepends package_DIR (if specified) to the -L and -I results from
the package's config tool instead of overriding them.
I also did a little reformatting. It was ugly.
Diffs
-
branches/11/autoconf/ast_ext_tool_check.m4 414212
Diff: https://reviewboard.asterisk.org/r/3550/diff/
Testing
---
Tested with cross compile for armv7hl platform (package paths specified) and
with native x86_64 compile (no package paths specified).
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3664: build: Disable AST_FORTIFY_SOURCE if DONT_OPTIMIZE is selected.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3664/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- AST_FORTIFY_SOURCE is automatically set in ./Makefile even if DONT_OPTIMIZE is set in menuselect. This causes gcc to complain that _FORTIFY_SOURCE requires optimization. You can specify make AST_FORTIFY_SOURCE='' but I always forget. This patch moves the set of AST_FORTIFY_SOURCE to Makefile.rules and only sets it if DONT_OPTIMIZE is no. The move is necessary because the top-level Makefile doesn't include menuselect.makeopts. This doesn't solve the entire problem however because res_config_mysql seems to force _FORTIFY_SOURCE so res_config_mysql has to be disabled for now if DONT_OPTIMIZE is set. Diffs - branches/12/Makefile.rules 416992 branches/12/Makefile 416992 Diff: https://reviewboard.asterisk.org/r/3664/diff/ Testing --- Tested compile both with and without the optimize flag set and insured that the proper combination of _FORTIFY_SOURCE and -O were set. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3664: build: Disable AST_FORTIFY_SOURCE if DONT_OPTIMIZE is selected.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3664/ --- (Updated June 22, 2014, 3:46 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 417016 Repository: Asterisk Description --- AST_FORTIFY_SOURCE is automatically set in ./Makefile even if DONT_OPTIMIZE is set in menuselect. This causes gcc to complain that _FORTIFY_SOURCE requires optimization. You can specify make AST_FORTIFY_SOURCE='' but I always forget. This patch moves the set of AST_FORTIFY_SOURCE to Makefile.rules and only sets it if DONT_OPTIMIZE is no. The move is necessary because the top-level Makefile doesn't include menuselect.makeopts. This doesn't solve the entire problem however because res_config_mysql seems to force _FORTIFY_SOURCE so res_config_mysql has to be disabled for now if DONT_OPTIMIZE is set. Diffs - branches/12/Makefile.rules 416992 branches/12/Makefile 416992 Diff: https://reviewboard.asterisk.org/r/3664/diff/ Testing --- Tested compile both with and without the optimize flag set and insured that the proper combination of _FORTIFY_SOURCE and -O were set. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3670: ao2_container node object ignores REF_DEBUG in all places except one
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3670/#review12291 --- Ship it! Ship It! - George Joseph On June 24, 2014, 12:28 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3670/ --- (Updated June 24, 2014, 12:28 a.m.) Review request for Asterisk Developers, George Joseph, Matt Jordan, and rmudgett. Bugs: ASTERISK-23922 https://issues.asterisk.org/jira/browse/ASTERISK-23922 Repository: Asterisk Description --- Almost every reference operation against container node's uses __ao2_alloc or __ao2_ref, thereby preventing ref logging for the nodes. One node reference is released with ao2_t_ref, causing refcounter.py to falsely report skews and leaks for many nodes. Diffs - /branches/12/main/astobj2_container.c 417180 Diff: https://reviewboard.asterisk.org/r/3670/diff/ Testing --- Tested against media_formats-reviewed-trunk (that's where I found the issue). Verified no container nodes appeared in the REF_DEBUG. Thanks, Corey Farrell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3705: res_pjsip_dialog_info_body_generator: Add dialog-info+xml implementation and tweak res_pjsip_pubsub to support it.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3705/#review12449 --- Ship it! Tested on my Grandstreams. Ship It! - George Joseph On July 2, 2014, 2:57 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3705/ --- (Updated July 2, 2014, 2:57 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-21443 https://issues.asterisk.org/jira/browse/ASTERISK-21443 Repository: Asterisk Description --- This change does the following: 1. Provides the underlying subscription to body generators for extension state by placing it in the data structure. 2. Makes the ast_sip_presence_xml_create_node function optionally add the created node to a parent node. 3. Adds dialog as a supported event to res_pjsip_exten_state. 4. Adds a res_pjsip_dialog_info_body_generator module for generating the dialog-info+xml body for NOTIFY messages. Diffs - /branches/12/res/res_pjsip_exten_state.c 417756 /branches/12/res/res_pjsip_dialog_info_body_generator.c PRE-CREATION /branches/12/res/res_pjsip/presence_xml.c 417756 /branches/12/include/asterisk/res_pjsip_presence_xml.h 417756 /branches/12/include/asterisk/res_pjsip_body_generator_types.h 417756 Diff: https://reviewboard.asterisk.org/r/3705/diff/ Testing --- Used a Grandstream phone to subscribe and watched NOTIFY messages go to it as I did things. Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3716: Weak Reference Containers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/ --- Review request for Asterisk Developers, Corey Farrell and rmudgett. Repository: Asterisk Description --- Weak Reference Containers are hash containers that don't maintain references to the objects they contain. Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't decrease it. Sounds simple but because the container doesn't have a reference to the object, it's entirely possible that the objects could be destroyed while still in the container. Therefore much of the work in this patch is making sure that if the object is destroyed, it's properly cleaned out of any weak reference containers that it may have been in. This patch is almost 6000 lines but half of it is units tests...functional, performance, and stress. Richard: I apologize in advance...I undid some of the refactor undos you had me do earlier. :) Diffs - branches/12/utils/Makefile 418136 branches/12/tests/test_astobj2_weak.c PRE-CREATION branches/12/tests/test_astobj2_torture.c PRE-CREATION branches/12/tests/test_astobj2_thrash.c 418136 branches/12/tests/test_astobj2.c 418136 branches/12/main/astobj2_weak.c PRE-CREATION branches/12/main/astobj2_rbtree.c 418136 branches/12/main/astobj2_private.h 418136 branches/12/main/astobj2_hash_private.h PRE-CREATION branches/12/main/astobj2_hash.c 418136 branches/12/main/astobj2_container_private.h 418136 branches/12/main/astobj2_container.c 418136 branches/12/main/astobj2.c 418136 branches/12/include/asterisk/astobj2.h 418136 Diff: https://reviewboard.asterisk.org/r/3716/diff/ Testing --- A whole slew of unit tests were added and the existing astobj2 tests were modified to also test weak containers. All passed. A torture test was added that tests multiple threads accessing multiple containers for weak, hash and rbtree containers. All passed. The full Testsuite was run with the same number of tests passing that passed before the patch. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers
On July 15, 2014, 2 p.m., Corey Farrell wrote: The ability to add an object to multiple weak reference containers is concerning to me. I think this adds unnecessary complexity/risk to astobj2. If you could tell me a use case where this amount of freedom is required I might be convinced otherwise. Otherwise I'd feel much better with a more limited ability to use weak reference containers: 1. Only allow the caller of ao2_alloc to add the object to a weak container, and only one container. 2. Insertion to the weak container should only happen before the object is reachable outside the local scope. 3. The weak container would be a private detail to the unit that owns it. All other code would use a method to retrieve references from that container. These two rules would greatly reduce the possibility of races, which reduces the amount of work needed to avoid races. In particular rule #2 would allow ao2_ref to only perform locking during ao2_ref(obj, -1). I had started work on this in r3463 but discarded it when you started splitting astobj2.c. We're going to disgree. :) I think this is a pretty severe restriction and it would be more complex to enforce the restriction than to safely allow it. How about this... If you can break it and I can't reasonably fix it, I'll back off. I know you're busy so just write a pseudo code test case, or just give me an idea for one and I'll create a unit test for it. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/#review12583 --- On July 7, 2014, 10:43 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/ --- (Updated July 7, 2014, 10:43 p.m.) Review request for Asterisk Developers, Corey Farrell and rmudgett. Repository: Asterisk Description --- Weak Reference Containers are hash containers that don't maintain references to the objects they contain. Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't decrease it. Sounds simple but because the container doesn't have a reference to the object, it's entirely possible that the objects could be destroyed while still in the container. Therefore much of the work in this patch is making sure that if the object is destroyed, it's properly cleaned out of any weak reference containers that it may have been in. This patch is almost 6000 lines but half of it is units tests...functional, performance, and stress. Richard: I apologize in advance...I undid some of the refactor undos you had me do earlier. :) Diffs - branches/12/utils/Makefile 418136 branches/12/tests/test_astobj2_weak.c PRE-CREATION branches/12/tests/test_astobj2_torture.c PRE-CREATION branches/12/tests/test_astobj2_thrash.c 418136 branches/12/tests/test_astobj2.c 418136 branches/12/main/astobj2_weak.c PRE-CREATION branches/12/main/astobj2_rbtree.c 418136 branches/12/main/astobj2_private.h 418136 branches/12/main/astobj2_hash_private.h PRE-CREATION branches/12/main/astobj2_hash.c 418136 branches/12/main/astobj2_container_private.h 418136 branches/12/main/astobj2_container.c 418136 branches/12/main/astobj2.c 418136 branches/12/include/asterisk/astobj2.h 418136 Diff: https://reviewboard.asterisk.org/r/3716/diff/ Testing --- A whole slew of unit tests were added and the existing astobj2 tests were modified to also test weak containers. All passed. A torture test was added that tests multiple threads accessing multiple containers for weak, hash and rbtree containers. All passed. The full Testsuite was run with the same number of tests passing that passed before the patch. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers
On July 24, 2014, 3:01 p.m., rmudgett wrote: You are affecting the lifetime of the continer node object without protecting it. It can now potentially be used outside the lifetime of its container as part of the held object. The container node and the continer ponter in the container node would be stale. This can happen when the container is being destroyed at the same time as the object within the container is being destroyed. You would need to give the object a reference to the container node and the container itself. The reference to the container prevents the nasty destruction race collision. Yes, all objects in the weak container would need to die before the container could self destruct or the weak container can be explicitly emptied to get rid of it earlier. However, since they are expected to be used as global containers that is not a problem at shutdown. It would be much simpler if the held object just contained a list of the weak containers with a reference instead of the container's node object. The ao2 object's clean up could simply use ao2_unlink() to remove itself from the weak container. The use of the container node to pull double duty adds just too much coupling and complexity. Weak containers can only return an object after successfully getting its reference (ao2_ref(obj, +1) returning a non-negative value). If it fails internal_ao2_traverse() must go on to the next object. An object can't use a list of containers as this would mean the container could only be in one object's list at a time. I'd have to use a container of containers which would be significant overhead. That's why I went with node since a node can only be associated with one object. Having a reference to the container makes sense though. I think Corey suggested that a while back. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/#review12860 --- On July 7, 2014, 10:43 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/ --- (Updated July 7, 2014, 10:43 p.m.) Review request for Asterisk Developers, Corey Farrell and rmudgett. Repository: Asterisk Description --- Weak Reference Containers are hash containers that don't maintain references to the objects they contain. Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't decrease it. Sounds simple but because the container doesn't have a reference to the object, it's entirely possible that the objects could be destroyed while still in the container. Therefore much of the work in this patch is making sure that if the object is destroyed, it's properly cleaned out of any weak reference containers that it may have been in. This patch is almost 6000 lines but half of it is units tests...functional, performance, and stress. Richard: I apologize in advance...I undid some of the refactor undos you had me do earlier. :) Diffs - branches/12/utils/Makefile 418136 branches/12/tests/test_astobj2_weak.c PRE-CREATION branches/12/tests/test_astobj2_torture.c PRE-CREATION branches/12/tests/test_astobj2_thrash.c 418136 branches/12/tests/test_astobj2.c 418136 branches/12/main/astobj2_weak.c PRE-CREATION branches/12/main/astobj2_rbtree.c 418136 branches/12/main/astobj2_private.h 418136 branches/12/main/astobj2_hash_private.h PRE-CREATION branches/12/main/astobj2_hash.c 418136 branches/12/main/astobj2_container_private.h 418136 branches/12/main/astobj2_container.c 418136 branches/12/main/astobj2.c 418136 branches/12/include/asterisk/astobj2.h 418136 Diff: https://reviewboard.asterisk.org/r/3716/diff/ Testing --- A whole slew of unit tests were added and the existing astobj2 tests were modified to also test weak containers. All passed. A torture test was added that tests multiple threads accessing multiple containers for weak, hash and rbtree containers. All passed. The full Testsuite was run with the same number of tests passing that passed before the patch. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers
On July 24, 2014, 3:01 p.m., rmudgett wrote: You are affecting the lifetime of the continer node object without protecting it. It can now potentially be used outside the lifetime of its container as part of the held object. The container node and the continer ponter in the container node would be stale. This can happen when the container is being destroyed at the same time as the object within the container is being destroyed. You would need to give the object a reference to the container node and the container itself. The reference to the container prevents the nasty destruction race collision. Yes, all objects in the weak container would need to die before the container could self destruct or the weak container can be explicitly emptied to get rid of it earlier. However, since they are expected to be used as global containers that is not a problem at shutdown. It would be much simpler if the held object just contained a list of the weak containers with a reference instead of the container's node object. The ao2 object's clean up could simply use ao2_unlink() to remove itself from the weak container. The use of the container node to pull double duty adds just too much coupling and complexity. Weak containers can only return an object after successfully getting its reference (ao2_ref(obj, +1) returning a non-negative value). If it fails internal_ao2_traverse() must go on to the next object. George Joseph wrote: An object can't use a list of containers as this would mean the container could only be in one object's list at a time. I'd have to use a container of containers which would be significant overhead. That's why I went with node since a node can only be associated with one object. Having a reference to the container makes sense though. I think Corey suggested that a while back. rmudgett wrote: I think you are making an invalid assumption about ao2 containers. An ao2 container can hold the same object more than once. The AO2_CONTAINER_ALLOC_OPT_DUPS_xxx options specify how a container handles duplicates. Other than the AO2_CONTAINER_ALLOC_OPT_DUPS_ALLOW option, the container must be sorted. ao2_unlink() only unlinks the object once if it is in the container. If it is in the container several times, you must ao2_unlink it again. An object can have the same container listed in its list of weak containers. Once for every time it is in that container. I'm only talking about a simple list just like you are doing with the container nodes. I'm not talking about an ao2 container to hold them as that would be a lot of overhead and overkill. Object is a member of these weak reference containers: channels stasis-controlled channels The above object is in the channels container twice and once in the stasis-controlled container. When the object is destroyed it unlinks itself from all containers listed. I understand how the containers work, it's linked lists that are the issue. The container needs a list entry structure to participate in a linked list, no? That means that that container can only be in 1 linked list that uses that list entry at a time. Hence it can't be in more than 1 object's list at a time. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/#review12860 --- On July 7, 2014, 10:43 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/ --- (Updated July 7, 2014, 10:43 p.m.) Review request for Asterisk Developers, Corey Farrell and rmudgett. Repository: Asterisk Description --- Weak Reference Containers are hash containers that don't maintain references to the objects they contain. Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't decrease it. Sounds simple but because the container doesn't have a reference to the object, it's entirely possible that the objects could be destroyed while still in the container. Therefore much of the work in this patch is making sure that if the object is destroyed, it's properly cleaned out of any weak reference containers that it may have been in. This patch is almost 6000 lines but half of it is units tests...functional, performance, and stress. Richard: I apologize in advance...I undid some of the refactor undos you had me do earlier. :) Diffs - branches/12/utils/Makefile 418136 branches/12/tests/test_astobj2_weak.c PRE-CREATION branches/12/tests/test_astobj2_torture.c PRE-CREATION branches/12/tests
Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers
On July 24, 2014, 3:01 p.m., rmudgett wrote: You are affecting the lifetime of the continer node object without protecting it. It can now potentially be used outside the lifetime of its container as part of the held object. The container node and the continer ponter in the container node would be stale. This can happen when the container is being destroyed at the same time as the object within the container is being destroyed. You would need to give the object a reference to the container node and the container itself. The reference to the container prevents the nasty destruction race collision. Yes, all objects in the weak container would need to die before the container could self destruct or the weak container can be explicitly emptied to get rid of it earlier. However, since they are expected to be used as global containers that is not a problem at shutdown. It would be much simpler if the held object just contained a list of the weak containers with a reference instead of the container's node object. The ao2 object's clean up could simply use ao2_unlink() to remove itself from the weak container. The use of the container node to pull double duty adds just too much coupling and complexity. Weak containers can only return an object after successfully getting its reference (ao2_ref(obj, +1) returning a non-negative value). If it fails internal_ao2_traverse() must go on to the next object. George Joseph wrote: An object can't use a list of containers as this would mean the container could only be in one object's list at a time. I'd have to use a container of containers which would be significant overhead. That's why I went with node since a node can only be associated with one object. Having a reference to the container makes sense though. I think Corey suggested that a while back. rmudgett wrote: I think you are making an invalid assumption about ao2 containers. An ao2 container can hold the same object more than once. The AO2_CONTAINER_ALLOC_OPT_DUPS_xxx options specify how a container handles duplicates. Other than the AO2_CONTAINER_ALLOC_OPT_DUPS_ALLOW option, the container must be sorted. ao2_unlink() only unlinks the object once if it is in the container. If it is in the container several times, you must ao2_unlink it again. An object can have the same container listed in its list of weak containers. Once for every time it is in that container. I'm only talking about a simple list just like you are doing with the container nodes. I'm not talking about an ao2 container to hold them as that would be a lot of overhead and overkill. Object is a member of these weak reference containers: channels stasis-controlled channels The above object is in the channels container twice and once in the stasis-controlled container. When the object is destroyed it unlinks itself from all containers listed. George Joseph wrote: I understand how the containers work, it's linked lists that are the issue. The container needs a list entry structure to participate in a linked list, no? That means that that container can only be in 1 linked list that uses that list entry at a time. Hence it can't be in more than 1 object's list at a time. Let me rephrase... the container can't be in multiple object's lists. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/#review12860 --- On July 7, 2014, 10:43 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3716/ --- (Updated July 7, 2014, 10:43 p.m.) Review request for Asterisk Developers, Corey Farrell and rmudgett. Repository: Asterisk Description --- Weak Reference Containers are hash containers that don't maintain references to the objects they contain. Basically, ao2_link() doesn't increase the ref count and ao2_unlink() doesn't decrease it. Sounds simple but because the container doesn't have a reference to the object, it's entirely possible that the objects could be destroyed while still in the container. Therefore much of the work in this patch is making sure that if the object is destroyed, it's properly cleaned out of any weak reference containers that it may have been in. This patch is almost 6000 lines but half of it is units tests...functional, performance, and stress. Richard: I apologize in advance...I undid some of the refactor undos you had me do earlier. :) Diffs - branches/12/utils/Makefile 418136 branches/12/tests
Re: [asterisk-dev] [Code Review] 3716: Weak Reference Containers
On July 24, 2014, 3:01 p.m., rmudgett wrote:
You are affecting the lifetime of the continer node object without
protecting it. It can now potentially be used outside the lifetime of its
container as part of the held object. The container node and the continer
ponter in the container node would be stale. This can happen when the
container is being destroyed at the same time as the object within the
container is being destroyed. You would need to give the object a
reference to the container node and the container itself. The reference to
the container prevents the nasty destruction race collision. Yes, all
objects in the weak container would need to die before the container could
self destruct or the weak container can be explicitly emptied to get rid of
it earlier. However, since they are expected to be used as global
containers that is not a problem at shutdown.
It would be much simpler if the held object just contained a list of the
weak containers with a reference instead of the container's node object.
The ao2 object's clean up could simply use ao2_unlink() to remove itself
from the weak container. The use of the container node to pull double duty
adds just too much coupling and complexity.
Weak containers can only return an object after successfully getting its
reference (ao2_ref(obj, +1) returning a non-negative value). If it fails
internal_ao2_traverse() must go on to the next object.
George Joseph wrote:
An object can't use a list of containers as this would mean the container
could only be in one object's list at a time. I'd have to use a container of
containers which would be significant overhead. That's why I went with node
since a node can only be associated with one object. Having a reference to
the container makes sense though. I think Corey suggested that a while back.
rmudgett wrote:
I think you are making an invalid assumption about ao2 containers. An
ao2 container can hold the same object more than once. The
AO2_CONTAINER_ALLOC_OPT_DUPS_xxx options specify how a container handles
duplicates. Other than the AO2_CONTAINER_ALLOC_OPT_DUPS_ALLOW option, the
container must be sorted. ao2_unlink() only unlinks the object once if it is
in the container. If it is in the container several times, you must
ao2_unlink it again.
An object can have the same container listed in its list of weak
containers. Once for every time it is in that container. I'm only talking
about a simple list just like you are doing with the container nodes. I'm
not talking about an ao2 container to hold them as that would be a lot of
overhead and overkill.
Object is a member of these weak reference containers:
channels
stasis-controlled
channels
The above object is in the channels container twice and once in the
stasis-controlled container. When the object is destroyed it unlinks itself
from all containers listed.
George Joseph wrote:
I understand how the containers work, it's linked lists that are the
issue. The container needs a list entry structure to participate in a linked
list, no? That means that that container can only be in 1 linked list that
uses that list entry at a time. Hence it can't be in more than 1 object's
list at a time.
George Joseph wrote:
Let me rephrase... the container can't be in multiple object's lists.
rmudgett wrote:
A list entry node needs to be allocated for every time it is put on an
objects weak container membership list.
Allocate a struct like this for each entry added to the objects
membership list when it is added.
struct weak_membership_node {
AST_LIST_ENTRY(weak_membership_node) next;
/*! Holds a reference to the weak container the object is in. */
struct ao2_container *member_of;
}
How is this any better than just using the node? Concurrency issues aside,
it's already allocated and it saves having to do a traverse on the container
just to find it and unlink it.
- George
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3716/#review12860
---
On July 7, 2014, 10:43 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3716/
---
(Updated July 7, 2014, 10:43 p.m.)
Review request for Asterisk Developers, Corey Farrell and rmudgett.
Repository: Asterisk
Description
---
Weak Reference Containers are hash containers that don't maintain references
to the objects they contain.
Basically, ao2_link() doesn't increase the ref count
[asterisk-dev] [Code Review] 3891: Fix Lua regression caused by me fixing ASTERISK-23818 incorrectly.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3891/ --- Review request for Asterisk Developers and Matt Jordan. Bugs: ASTERISK-23818 https://issues.asterisk.org/jira/browse/ASTERISK-23818 Repository: Asterisk Description --- ASTERISK-23818 (lua contexts being overwritten by contexts of the same name in pbx_config) surfaced because pbx_lua, having the AST_MODFLAG_GLOBAL_SYMBOLS set, was always force loaded before pbx_config. Since I couldn't find any reason for pbx_lua to export it's symbols to the rest of Asterisk, I simply changed the flag to AST_MODFLAG_DEFAULT. Problem solved. What I didn't realize was that the symbols need to be exported not because Asterisk needs them but because any external Lua modules like luasql.mysql need the base Lua language APIs exported (ASTERISK-17279). Back to ASTERISK-23818... It looks like there's an issue in pbx.c where context_merge was only merging includes, switches and ignore patterns if the context was already existing AND has extensions, or if the context was brand new. If pbx_lua is loaded before pbx_config, the context will exist BUT pbx_lua, being implemented as a switch, will never place extensions in it, just the switch statement. The result is that when pbx_config loads, it never merges the switch statement created by pbx_lua into the final context. This patch sets pbx_lua's modflag back to AST_MODFLAG_GLOBAL_SYMBOLS and adds an else if in context_merge that catches the case where an existing context has includes, switchs or ingore patterns but no actual extensions. For later... dialplan show always shows extensions, includes, then switches but pbx_find_extension actually processes extensions, switches then includes. Which is correct? Also, it appears that switches and includes are always inserted at the tail of their respective lists, but the lists are searched in reverse. The result is that the last module loaded is searched for a matching extension first. Is this expected behavior? This patch needs to be applied from 1.8 through trunk. Diffs - branches/1.8/pbx/pbx_lua.c 420123 branches/1.8/main/pbx.c 420123 Diff: https://reviewboard.asterisk.org/r/3891/diff/ Testing --- Ran the TestSuite before and after with the same results. 461 run, 397 passed, 64 failed. Confirmed that external Lua modules properly initialize. I used luasql.mysql as a test. Ran tests with various merge scenarios between pbx_config, pbx_lua and pbx_ael to make sure that where contexts overlap, includes, switches, and ingore patterns are preserved in the merge process. Extensions are different... pbx_config and pbx_ael share the same global dialplan so the first definition of context/exten/priority wins and subsequent are rejected. pbx_lua maintains it's own environment so even if it happens to define the same context/exten as pbx_config or pbx_ael it'll never be called because pbx_find_extension will never search pbx_lua unless the global table is exhausted. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3891: Fix Lua regression caused by me fixing ASTERISK-23818 incorrectly.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3891/ --- (Updated Aug. 6, 2014, 11:05 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and Matt Jordan. Changes --- Committed in revision 420146 Bugs: ASTERISK-23818 https://issues.asterisk.org/jira/browse/ASTERISK-23818 Repository: Asterisk Description --- ASTERISK-23818 (lua contexts being overwritten by contexts of the same name in pbx_config) surfaced because pbx_lua, having the AST_MODFLAG_GLOBAL_SYMBOLS set, was always force loaded before pbx_config. Since I couldn't find any reason for pbx_lua to export it's symbols to the rest of Asterisk, I simply changed the flag to AST_MODFLAG_DEFAULT. Problem solved. What I didn't realize was that the symbols need to be exported not because Asterisk needs them but because any external Lua modules like luasql.mysql need the base Lua language APIs exported (ASTERISK-17279). Back to ASTERISK-23818... It looks like there's an issue in pbx.c where context_merge was only merging includes, switches and ignore patterns if the context was already existing AND has extensions, or if the context was brand new. If pbx_lua is loaded before pbx_config, the context will exist BUT pbx_lua, being implemented as a switch, will never place extensions in it, just the switch statement. The result is that when pbx_config loads, it never merges the switch statement created by pbx_lua into the final context. This patch sets pbx_lua's modflag back to AST_MODFLAG_GLOBAL_SYMBOLS and adds an else if in context_merge that catches the case where an existing context has includes, switchs or ingore patterns but no actual extensions. For later... dialplan show always shows extensions, includes, then switches but pbx_find_extension actually processes extensions, switches then includes. Which is correct? Also, it appears that switches and includes are always inserted at the tail of their respective lists, but the lists are searched in reverse. The result is that the last module loaded is searched for a matching extension first. Is this expected behavior? This patch needs to be applied from 1.8 through trunk. Diffs - branches/1.8/pbx/pbx_lua.c 420123 branches/1.8/main/pbx.c 420123 Diff: https://reviewboard.asterisk.org/r/3891/diff/ Testing --- Ran the TestSuite before and after with the same results. 461 run, 397 passed, 64 failed. Confirmed that external Lua modules properly initialize. I used luasql.mysql as a test. Ran tests with various merge scenarios between pbx_config, pbx_lua and pbx_ael to make sure that where contexts overlap, includes, switches, and ingore patterns are preserved in the merge process. Extensions are different... pbx_config and pbx_ael share the same global dialplan so the first definition of context/exten/priority wins and subsequent are rejected. pbx_lua maintains it's own environment so even if it happens to define the same context/exten as pbx_config or pbx_ael it'll never be called because pbx_find_extension will never search pbx_lua unless the global table is exhausted. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3919: func_config: Change 'Not Found' message from ERROR to DEBUG
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3919/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- This one's been bugging me forever... When you call the CONFIG dialplan function with the name of a variable that doesn't exist in the target context you get an ERROR. This does nothing but clutter up the logs with messages that may be perfectly acceptable. Just because a variable wasn't in the context doesn't mean it's an error. Maybe it's optional or just needs to be defaulted or ignored. This patch changes the log level from ERROR to DEBUG. If a dialplan developer wants to debug their dialplan they still can. Diffs - branches/12/funcs/func_config.c 421326 Diff: https://reviewboard.asterisk.org/r/3919/diff/ Testing --- Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3919: func_config: Change 'Not Found' message from ERROR to DEBUG
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3919/ --- (Updated Aug. 18, 2014, 12:47 p.m.) Review request for Asterisk Developers. Changes --- Updated to ast_debug(1... and set branch to 1.8. Will merge forward. Repository: Asterisk Description --- This one's been bugging me forever... When you call the CONFIG dialplan function with the name of a variable that doesn't exist in the target context you get an ERROR. This does nothing but clutter up the logs with messages that may be perfectly acceptable. Just because a variable wasn't in the context doesn't mean it's an error. Maybe it's optional or just needs to be defaulted or ignored. This patch changes the log level from ERROR to DEBUG. If a dialplan developer wants to debug their dialplan they still can. Diffs (updated) - branches/12/funcs/func_config.c 421326 Diff: https://reviewboard.asterisk.org/r/3919/diff/ Testing --- Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3919: func_config: Change 'Not Found' message from ERROR to DEBUG
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3919/ --- (Updated Aug. 18, 2014, 3:14 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 421327 Repository: Asterisk Description --- This one's been bugging me forever... When you call the CONFIG dialplan function with the name of a variable that doesn't exist in the target context you get an ERROR. This does nothing but clutter up the logs with messages that may be perfectly acceptable. Just because a variable wasn't in the context doesn't mean it's an error. Maybe it's optional or just needs to be defaulted or ignored. This patch changes the log level from ERROR to DEBUG. If a dialplan developer wants to debug their dialplan they still can. Diffs - branches/12/funcs/func_config.c 421326 Diff: https://reviewboard.asterisk.org/r/3919/diff/ Testing --- Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Although tab completion for confbridge mute and unmute show 'all' as a valid channel target, it was never implemented. This patch updates mute and unmute (both CLI and AMI) to allow the 'all' target. They now work as kick does. Since I was there, I made the channel name case-insensitive since kick was already case-insensitive. Diffs - branches/12/apps/app_confbridge.c 422052 Diff: https://reviewboard.asterisk.org/r/3944/diff/ Testing --- Tested that both CLI and AMI handle 'all' as a channel target for mute and unmute correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3931: runtests.py: -d (--dry-run)
On Aug. 25, 2014, 11:53 a.m., Matt Jordan wrote: How is this different than the -l option? Just to chip in... The -l option lists everything even though the test might not actually get run because of some constraint. I use something like the proposed dry-run so I can know how many tests will run before I actually run them. Makes it easier to know how far into the test run you are. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3931/#review13170 --- On Aug. 24, 2014, 11:44 a.m., Tzafrir Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3931/ --- (Updated Aug. 24, 2014, 11:44 a.m.) Review request for Asterisk Developers. Repository: testsuite Description --- Adds option -d (--dry-run) to runtests.py to just list the tests that will be run and not actually run them. Diffs - /asterisk/trunk/runtests.py 5520 Diff: https://reviewboard.asterisk.org/r/3931/diff/ Testing --- Thanks, Tzafrir Cohen -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/ --- (Updated Aug. 25, 2014, 5:53 p.m.) Review request for Asterisk Developers. Changes --- Updated to add participants as a target for mute/unmute/kick to take action against all non-admins. Repository: Asterisk Description --- Although tab completion for confbridge mute and unmute show 'all' as a valid channel target, it was never implemented. This patch updates mute and unmute (both CLI and AMI) to allow the 'all' target. They now work as kick does. Since I was there, I made the channel name case-insensitive since kick was already case-insensitive. Diffs (updated) - branches/12/apps/app_confbridge.c 422052 Diff: https://reviewboard.asterisk.org/r/3944/diff/ Testing (updated) --- Tested that both CLI and AMI handle 'all' and 'participants' as a channel target for mute, unmute and kick correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/ --- (Updated Aug. 26, 2014, 11:35 a.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's issues. Refactored the tests for all and participants so they're only done once. Repository: Asterisk Description --- Although tab completion for confbridge mute and unmute show 'all' as a valid channel target, it was never implemented. This patch updates mute and unmute (both CLI and AMI) to allow the 'all' target. They now work as kick does. Since I was there, I made the channel name case-insensitive since kick was already case-insensitive. Diffs (updated) - branches/12/apps/app_confbridge.c 422052 Diff: https://reviewboard.asterisk.org/r/3944/diff/ Testing --- Tested that both CLI and AMI handle 'all' and 'participants' as a channel target for mute, unmute and kick correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/ --- (Updated Aug. 26, 2014, 3:26 p.m.) Review request for Asterisk Developers. Changes --- Resolved more issues. Repository: Asterisk Description --- Although tab completion for confbridge mute and unmute show 'all' as a valid channel target, it was never implemented. This patch updates mute and unmute (both CLI and AMI) to allow the 'all' target. They now work as kick does. Since I was there, I made the channel name case-insensitive since kick was already case-insensitive. Diffs (updated) - branches/12/apps/app_confbridge.c 422068 Diff: https://reviewboard.asterisk.org/r/3944/diff/ Testing --- Tested that both CLI and AMI handle 'all' and 'participants' as a channel target for mute, unmute and kick correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/ --- (Updated Aug. 26, 2014, 4:43 p.m.) Review request for Asterisk Developers. Changes --- More updates. Repository: Asterisk Description --- Although tab completion for confbridge mute and unmute show 'all' as a valid channel target, it was never implemented. This patch updates mute and unmute (both CLI and AMI) to allow the 'all' target. They now work as kick does. Since I was there, I made the channel name case-insensitive since kick was already case-insensitive. Diffs (updated) - branches/12/apps/app_confbridge.c 422068 Diff: https://reviewboard.asterisk.org/r/3944/diff/ Testing --- Tested that both CLI and AMI handle 'all' and 'participants' as a channel target for mute, unmute and kick correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3944/ --- (Updated Aug. 26, 2014, 6:18 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 422090 Repository: Asterisk Description --- Although tab completion for confbridge mute and unmute show 'all' as a valid channel target, it was never implemented. This patch updates mute and unmute (both CLI and AMI) to allow the 'all' target. They now work as kick does. Since I was there, I made the channel name case-insensitive since kick was already case-insensitive. Diffs - branches/12/apps/app_confbridge.c 422068 Diff: https://reviewboard.asterisk.org/r/3944/diff/ Testing --- Tested that both CLI and AMI handle 'all' and 'participants' as a channel target for mute, unmute and kick correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3950: confbridge: Add 'Admin' parameter to join, leave, mute, unmute and talking events
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3950/ --- Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- Currently there's no way to tell if a user is an admin or not when receiving the join, leave, mute, unmute and talking events. This patch adds that capability. Diffs - branches/12/apps/confbridge/confbridge_manager.c 422108 branches/12/apps/app_confbridge.c 422108 Diff: https://reviewboard.asterisk.org/r/3950/diff/ Testing --- Checked that the AMI events had properly set Admin flags. All of the 11 eligible TestSuite confbridge tests passed although they don't test for the existence of the new flag yet. Nothing broke at least. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3950: confbridge: Add 'Admin' parameter to join, leave, mute, unmute and talking events
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3950/ --- (Updated Aug. 27, 2014, 10:56 a.m.) Review request for Asterisk Developers and rmudgett. Changes --- Addressed Richard's comments. Repository: Asterisk Description --- Currently there's no way to tell if a user is an admin or not when receiving the join, leave, mute, unmute and talking events. This patch adds that capability. Diffs (updated) - branches/12/apps/confbridge/confbridge_manager.c 422175 branches/12/apps/app_confbridge.c 422175 Diff: https://reviewboard.asterisk.org/r/3950/diff/ Testing --- Checked that the AMI events had properly set Admin flags. All of the 11 eligible TestSuite confbridge tests passed although they don't test for the existence of the new flag yet. Nothing broke at least. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3950: confbridge: Add 'Admin' parameter to join, leave, mute, unmute and talking events
On Aug. 27, 2014, 11:12 a.m., rmudgett wrote: branches/12/apps/app_confbridge.c, lines 1834-1835 https://reviewboard.asterisk.org/r/3950/diff/2/?file=66863#file66863line1834 This probably fits on one line now. Ends up at column 102. :) I'll leave it wrapped. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3950/#review13190 --- On Aug. 27, 2014, 10:56 a.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3950/ --- (Updated Aug. 27, 2014, 10:56 a.m.) Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- Currently there's no way to tell if a user is an admin or not when receiving the join, leave, mute, unmute and talking events. This patch adds that capability. Diffs - branches/12/apps/confbridge/confbridge_manager.c 422175 branches/12/apps/app_confbridge.c 422175 Diff: https://reviewboard.asterisk.org/r/3950/diff/ Testing --- Checked that the AMI events had properly set Admin flags. All of the 11 eligible TestSuite confbridge tests passed although they don't test for the existence of the new flag yet. Nothing broke at least. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3950: confbridge: Add 'Admin' parameter to join, leave, mute, unmute and talking events
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3950/ --- (Updated Aug. 27, 2014, 12:22 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and rmudgett. Changes --- Committed in revision 422176 Repository: Asterisk Description --- Currently there's no way to tell if a user is an admin or not when receiving the join, leave, mute, unmute and talking events. This patch adds that capability. Diffs - branches/12/apps/confbridge/confbridge_manager.c 422175 branches/12/apps/app_confbridge.c 422175 Diff: https://reviewboard.asterisk.org/r/3950/diff/ Testing --- Checked that the AMI events had properly set Admin flags. All of the 11 eligible TestSuite confbridge tests passed although they don't test for the existence of the new flag yet. Nothing broke at least. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3955: confbridge: Add Duration to ConfbridgeList event
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3955/ --- Review request for Asterisk Developers and rmudgett. Repository: Asterisk Description --- The ConfbridgeList event doesn't include how long the user has been a member of the conference. This patch adds Duration (seconds) which is based on user-chan-answertime. Diffs - branches/12/apps/app_confbridge.c 422257 Diff: https://reviewboard.asterisk.org/r/3955/diff/ Testing --- Looked at the output of a ConfbridgeList event to make sure Duration was being set correctly and no other fields were affected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3958: manager: Make WaitEvent action respect eventfilters
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3958/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- A WaitEvent issued via an http session isn't respecting eventfilters defined for the user. I just added a match_filter to the predicate that controls astman_append. Diffs - branches/12/main/manager.c 422277 Diff: https://reviewboard.asterisk.org/r/3958/diff/ Testing --- Verified that events to be filtered are indeed filtered and that events not supposed to be filtered aren't. I ran the manager TestSuite but 4 of 9 tests failed BEFORE my change. The same 4 failed after my change. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Thoughts on the future of phoneprov.
The last hurdle for me to adopt pjsip has been what to do with phoneprov and now that chan_sip has been moved to extended support, this has come to the front burner. I've been thinking of ways to make the existing res_phoneprov configurable to use either the existing users.conf/sip.conf infrastructure or pjsip. Maybe create a new res_pjsip_provisioning module that provides a new pjsip provisioning object which would be associated to an endpoint. It would contain mac address, profile, line keys, etc. Then res_phoneprov could be modified to use alternate config back-ends leaving the existing users.conf/sip.conf mechanism as the default (for now). Actually, with this approach, you could use multiple config back-ends at the same time. Thoughts? Other ideas? -- _ -- 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] 3955: confbridge: Add Duration to ConfbridgeList event
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3955/ --- (Updated Aug. 29, 2014, 4:27 p.m.) Review request for Asterisk Developers and rmudgett. Changes --- Updated name from Duration to AnsweredTime. Repository: Asterisk Description --- The ConfbridgeList event doesn't include how long the user has been a member of the conference. This patch adds Duration (seconds) which is based on user-chan-answertime. Diffs (updated) - branches/12/apps/app_confbridge.c 422257 Diff: https://reviewboard.asterisk.org/r/3955/diff/ Testing --- Looked at the output of a ConfbridgeList event to make sure Duration was being set correctly and no other fields were affected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3958: manager: Make WaitEvent action respect eventfilters
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3958/ --- (Updated Aug. 30, 2014, 12:19 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 422439 Repository: Asterisk Description --- A WaitEvent issued via an http session isn't respecting eventfilters defined for the user. I just added a match_filter to the predicate that controls astman_append. Diffs - branches/12/main/manager.c 422277 Diff: https://reviewboard.asterisk.org/r/3958/diff/ Testing --- Verified that events to be filtered are indeed filtered and that events not supposed to be filtered aren't. I ran the manager TestSuite but 4 of 9 tests failed BEFORE my change. The same 4 failed after my change. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3955: confbridge: Add Duration to ConfbridgeList event
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3955/ --- (Updated Aug. 30, 2014, 12:28 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and rmudgett. Changes --- Committed in revision 422444 Repository: Asterisk Description --- The ConfbridgeList event doesn't include how long the user has been a member of the conference. This patch adds Duration (seconds) which is based on user-chan-answertime. Diffs - branches/12/apps/app_confbridge.c 422257 Diff: https://reviewboard.asterisk.org/r/3955/diff/ Testing --- Looked at the output of a ConfbridgeList event to make sure Duration was being set correctly and no other fields were affected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_provider_set_defaults should be called by the provider's load_defaults callback to push the defaults. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. Since res_phoneprov is a module that may or may not be loaded, the apis are implemented as inlines that check that res_phoneprov is actually loaded before calling the concrete function. A new include file was added: include/asterisk/phoneprov.h res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_defaults() and load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_set_defaults and ast_phoneprov_add_extension apis. So... The provider creates 2 callback functions which call the ast_provider_set_defaults and ast_phoneprov_add_extension apis. It then calls ast_phoneprov_provider_register with those 2 callbacks. res_phoneprov then calls the callbacks to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_defaults and load_users callbacks. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callbacks. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 422556 branches/12/main/chanvars.c 422556 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 422556 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 3, 2014, 6:28 p.m.) Review request for Asterisk Developers. Changes --- Addressed some issues I found while writing the pjsip provider and removed some silliness around testing if the module is loaded. Repository: Asterisk Description --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_provider_set_defaults should be called by the provider's load_defaults callback to push the defaults. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. Since res_phoneprov is a module that may or may not be loaded, the apis are implemented as inlines that check that res_phoneprov is actually loaded before calling the concrete function. A new include file was added: include/asterisk/phoneprov.h res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_defaults() and load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_set_defaults and ast_phoneprov_add_extension apis. So... The provider creates 2 callback functions which call the ast_provider_set_defaults and ast_phoneprov_add_extension apis. It then calls ast_phoneprov_provider_register with those 2 callbacks. res_phoneprov then calls the callbacks to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_defaults and load_users callbacks. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callbacks. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs (updated) - branches/12/res/res_phoneprov.c 422556 branches/12/main/chanvars.c 422556 branches/12/include/asterisk/chanvars.h 422556 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). Two new pjsip.conf objects are defined by this module... 'phoneprov_default' which defines defaults for all users created by this provider. [myppdefaults] type=phoneprov_default PROFILE=grandstream ; required SERVER=myserver.example.com OTHERVAR=othervalue 'phoneprov_user' which defines each user to be exposed. [pp1000] type=phoneprov_user endpoint=ep1000 ; optional reference to an existing endpoint MAC=deadbeef4dad ; required PROFILE=grandstream2 ; overrides the default LINEKEYS=1 LINE=1 OTHERVAR=othervalue [pp1001] type=phoneprov_user endpoint=ep1001 ; optional reference to an existing endpoint MAC=deadf00d4dad ; required LINEKEYS=1 LINE=1 LABEL=1001 ; overrides pp1001 OTHERVAR=othervalue USERNAME, CALLERID, DISPLAY_NAME and SECRET are automatically pulled from endpoint and endpoint-auth if defined. LABEL is automatically set from the phoneprov_user id. ENDPOINT_ID, TRANSPORT_ID and AUTH_ID are automatically added. Any other variables defined are automatically passed through and are available for template substitution even if they're not one of the standard variables defined by res_phoneprov. Diffs - branches/12/res/res_pjsip_phoneprov_provider.c PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3976/diff/ Testing --- I'm already starting to convert sip peers to pjsip endpoints with no change to my Grandstream templates. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 5, 2014, 6:01 p.m.) Review request for Asterisk Developers. Changes --- Removed the load_defaults process. It's actually easier to let the provider load and apply it's own defaults. Cleaned up some error messages. Added language to phoneprov.conf.sample concerning defaults and providers. Repository: Asterisk Description (updated) --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. Since res_phoneprov is a module that may or may not be loaded, the apis are implemented as inlines that check that res_phoneprov is actually loaded before calling the concrete function. A new include file was added: include/asterisk/phoneprov.h res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_defaults() and load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_set_defaults and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs (updated) - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 422737 branches/12/main/chanvars.c 422737 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 422737 branches/12/configs/phoneprov.conf.sample 422737 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 5, 2014, 6:44 p.m.) Review request for Asterisk Developers. Changes --- Fixed some reference leaks. Refactored for default processing change in res_phoneprov. Added documentation to pjsip.conf.sample. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). Two new pjsip.conf objects are defined by this module... 'phoneprov_default' which defines defaults for all users created by this provider. [myppdefaults] type=phoneprov_default PROFILE=grandstream ; required SERVER=myserver.example.com OTHERVAR=othervalue 'phoneprov_user' which defines each user to be exposed. [pp1000] type=phoneprov_user endpoint=ep1000 ; optional reference to an existing endpoint MAC=deadbeef4dad ; required PROFILE=grandstream2 ; overrides the default LINEKEYS=1 LINE=1 OTHERVAR=othervalue [pp1001] type=phoneprov_user endpoint=ep1001 ; optional reference to an existing endpoint MAC=deadf00d4dad ; required LINEKEYS=1 LINE=1 LABEL=1001 ; overrides pp1001 OTHERVAR=othervalue USERNAME, CALLERID, DISPLAY_NAME and SECRET are automatically pulled from endpoint and endpoint-auth if defined. LABEL is automatically set from the phoneprov_user id. ENDPOINT_ID, TRANSPORT_ID and AUTH_ID are automatically added. Any other variables defined are automatically passed through and are available for template substitution even if they're not one of the standard variables defined by res_phoneprov. Diffs (updated) - branches/12/res/res_pjsip_phoneprov_provider.c PRE-CREATION branches/12/configs/pjsip.conf.sample 422737 Diff: https://reviewboard.asterisk.org/r/3976/diff/ Testing --- I'm already starting to convert sip peers to pjsip endpoints with no change to my Grandstream templates. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3986: config: bug: fix truncation of included config files on permissions error
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- ast_config_text_file_save() currently truncates include files as they are processed. If a subsequent include file or the main config file has a permissions error that prevents writing, earlier include files are left truncated resulting in a frantic search for backups. This patch causes ast_config_text_file_save to check for write access on all files before it truncates any of them. Will be applied 1.8 trunk. Diffs - branches/1.8/main/config.c 422882 Diff: https://reviewboard.asterisk.org/r/3986/diff/ Testing --- Tested normal access and verified that no files are truncated if any of the can't be written to. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3986: config: bug: fix truncation of included config files on permissions error
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986/ --- (Updated Sept. 9, 2014, 7:34 p.m.) Review request for Asterisk Developers. Changes --- Updated for Richard's comments. Repository: Asterisk Description --- ast_config_text_file_save() currently truncates include files as they are processed. If a subsequent include file or the main config file has a permissions error that prevents writing, earlier include files are left truncated resulting in a frantic search for backups. This patch causes ast_config_text_file_save to check for write access on all files before it truncates any of them. Will be applied 1.8 trunk. Diffs (updated) - branches/1.8/main/config.c 422882 Diff: https://reviewboard.asterisk.org/r/3986/diff/ Testing --- Tested normal access and verified that no files are truncated if any of the can't be written to. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3986: config: bug: fix truncation of included config files on permissions error
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986/ --- (Updated Sept. 10, 2014, 10:58 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 422900 Repository: Asterisk Description --- ast_config_text_file_save() currently truncates include files as they are processed. If a subsequent include file or the main config file has a permissions error that prevents writing, earlier include files are left truncated resulting in a frantic search for backups. This patch causes ast_config_text_file_save to check for write access on all files before it truncates any of them. Will be applied 1.8 trunk. Diffs - branches/1.8/main/config.c 422882 Diff: https://reviewboard.asterisk.org/r/3986/diff/ Testing --- Tested normal access and verified that no files are truncated if any of the can't be written to. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
Review request for Asterisk Developers and rmudgett.
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags controls whitespace and quote stripping.
\return The next token or NULL if done.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, enum ast_strsep_flags flags);
Diffs
-
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
On Sept. 11, 2014, 1:29 p.m., opticron wrote:
This new string parsing function could use unit tests.
Good point. I'll create a test for it.
On Sept. 11, 2014, 1:29 p.m., opticron wrote:
branches/12/main/utils.c, lines 1483-1486
https://reviewboard.asterisk.org/r/3989/diff/1/?file=67308#file67308line1483
The way this is written, a quoted section could be opened with ' and
closed with or vice-versa. Is that a desired behavior?
Yeah, I knew this going in but I'm not sure if it's worth the extra code to
keep track of complex open and close punctuation. I'll give another look
though maybe with some performance comparisons.
- George
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/#review13284
---
On Sept. 10, 2014, 5:45 p.m., George Joseph wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 10, 2014, 5:45 p.m.)
Review request for Asterisk Developers and rmudgett.
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags controls whitespace and quote stripping.
\return The next token or NULL if done.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, enum ast_strsep_flags flags);
Diffs
-
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 12, 2014, 12:12 a.m.)
Review request for Asterisk Developers and rmudgett.
Changes
---
Added better quote matching.
Added ability to automatically unescape results.
Added tests to test_strings.c
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags controls whitespace and quote stripping.
\return The next token or NULL if done.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, enum ast_strsep_flags flags);
Diffs (updated)
-
branches/12/tests/test_strings.c 422963
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 12, 2014, 2:04 p.m.)
Review request for Asterisk Developers and rmudgett.
Changes
---
Addressed Mark's comments.
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags controls whitespace and quote stripping.
\return The next token or NULL if done.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, enum ast_strsep_flags flags);
Diffs (updated)
-
branches/12/tests/test_strings.c 422963
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3993: config: bug: Fix SEGV in ast_category_insert when a matching category isn't found
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3993/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- If you call ast_category_insert with a match category that doesn't exist, the list traverse runs out of 'next' categories and you get a SEGV. This patch adds check for the end-of-list condition and changes the signature to return an int for success/failure indication instead of a void. The only consumer of this function is manager and it was also changed to use the return value. Diffs - branches/1.8/main/manager.c 423127 branches/1.8/main/config.c 423127 branches/1.8/include/asterisk/config.h 423127 Diff: https://reviewboard.asterisk.org/r/3993/diff/ Testing --- Tested with AMI UpdateConfig/newcat to make sure the proper value is returned in both positive and negative cases. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3993: config: bug: Fix SEGV in ast_category_insert when a matching category isn't found
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3993/ --- (Updated Sept. 15, 2014, 9:03 a.m.) Review request for Asterisk Developers. Changes --- Addressed Matt's comment. Repository: Asterisk Description --- If you call ast_category_insert with a match category that doesn't exist, the list traverse runs out of 'next' categories and you get a SEGV. This patch adds check for the end-of-list condition and changes the signature to return an int for success/failure indication instead of a void. The only consumer of this function is manager and it was also changed to use the return value. Diffs (updated) - branches/1.8/main/manager.c 423127 branches/1.8/main/config.c 423127 branches/1.8/include/asterisk/config.h 423127 Diff: https://reviewboard.asterisk.org/r/3993/diff/ Testing --- Tested with AMI UpdateConfig/newcat to make sure the proper value is returned in both positive and negative cases. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 15, 2014, 1:43 p.m.)
Review request for Asterisk Developers and rmudgett.
Changes
---
More updates.
Repository: Asterisk
Description (updated)
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags Controls post-processing of the result.
AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
AST_STRSEP_STRIP does a trim then strips the outermost quotes. You may want
to trim again after the strip. Just OR both the TRIM and STRIP flags.
AST_STRSEP_UNESCAPE unescapes '\' sequences.
AST_STRSEP_ALL does all of the above processing.
\return The next token or NULL if done or if there are more than 8 levels of
nested quotes.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result and have '\' sequences unescaped.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, uint32_t flags);
Diffs (updated)
-
branches/12/tests/test_strings.c 422963
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Tue, Sep 16, 2014 at 1:48 PM, Matthew Jordan mjor...@digium.com wrote: And there was much rejoicing To summarize: * A comparison of management platforms has been done. Barring a giant catastrophe or some insane limitation, we're going to go simple here and stick with gitolite. Reasoning is on the wiki page. The only thing I'd suggest is to use CGit instead of gitweb for the web interface. It's got richer capabilities and works great with gitolite. Once it's initially configured you don't ever have to touch it. -- _ -- 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] 3998: res_pjsip: ami: Fix error in AMI output when no transport is associated to an endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3998/ --- Review request for Asterisk Developers and Kevin Harwell. Bugs: 24331 https://issues.asterisk.org/jira/browse/24331 Repository: Asterisk Description --- When no transport is associated to an endpoint, the AMI output for PJSIPShowEndpoint indicates an error instead of silently ignoring the missing transport. This patch causes the error to appear only if a transport was specified on the endpoint and the transport doesn't exist. It also fixes an issue with counting the objects that were actually found. Diffs - branches/12/res/res_pjsip_endpoint_identifier_ip.c 423274 branches/12/res/res_pjsip/pjsip_configuration.c 423274 branches/12/res/res_pjsip/location.c 423274 branches/12/res/res_pjsip/config_transport.c 423274 branches/12/res/res_pjsip/config_auth.c 423274 branches/12/include/asterisk/res_pjsip.h 423274 Diff: https://reviewboard.asterisk.org/r/3998/diff/ Testing --- Checked the output of PJSIPShowEndpoint to make sure the error appears (or doesn't) correctly and that ListItems is set correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3995: res_pjsip_endpoint_identifier_ip: Can't parse identify with match value containing CIDR
On Sept. 16, 2014, 12:16 p.m., Scott Griepentrog wrote: /branches/12/res/res_pjsip_endpoint_identifier_ip.c, lines 162-166 https://reviewboard.asterisk.org/r/3995/diff/1/?file=67358#file67358line162 The implementation of ast_append_ha supports comma separated values, but pjsip.conf.sample doesn't clearly indicate support nor lack of support for commas, although it does show examples of multiple match lines to accomplish the same thing. Assuming that we are expressly not going to support something like this: match=sip.example.com,1.2.3.4/10 Then this code is fine, otherwise would need to be reworked to parse the , values out first. Jonathan Rose wrote: Personally, I feel inclined to either not allow commas in the case of the example you mentioned or else perform comma separation while parsing the config rather than allow ast_apply_ha handle it. Right now if that were used I think it would fail since ast_apply_ha doesn't do host name matching... and otherwise we'd have an issue when people try to use comma separation with anything other than a collection of host addresses with at least one of them containing the CIDR pattern. I'll defer that decision to someone else though. Neither is particularly difficult to implement, I'm just not sure which is preferred. Unless it's significantly more work, I think the case of mixed formats has to be handled which means handling the comma separation while parsing the config. From a user perspective, it'd be weird to have a restriction that only formats of the same type can be used in a comma separated list. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3995/#review13315 --- On Sept. 15, 2014, 12:52 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3995/ --- (Updated Sept. 15, 2014, 12:52 p.m.) Review request for Asterisk Developers and Joshua Colp. Bugs: ASTERISK-24290 https://issues.asterisk.org/jira/browse/ASTERISK-24290 Repository: Asterisk Description --- Using a value such as '10.24.0.0/16' would fail to match because ast_sockaddr_resolve can only parse the following formats: * hostname:port * host.example.com:port * a.b.c.d * a.b.c.d:port * a:b:c:...:d * [a:b:c:...:d] * [a:b:c:...:d]:port When the format doesn't match one of these, the function fails and we bail. To get around this, I simply checked for the presence of a '/' in the identify string and used ast_append_ha directly with the address if it was present. Diffs - /branches/12/res/res_pjsip_endpoint_identifier_ip.c 423062 Diff: https://reviewboard.asterisk.org/r/3995/diff/ Testing --- Used CLI command 'pjsip show endpoint 1603' with an endpoint that had the following identifier: [1603] type=identify match=10.24.18.13/16 endpoint=1603 Before, the address would fail to parse and the command would show no identifier After, the address would parse correctly and show '10.24.0.0/16' for the identifier as seen in: Endpoint: 1603/1603Not in use 0 of inf Aor: 1603 5 Contact: 1603/sip:1603@10.24.18.13:5060;obUnknown nan Identify: 10.24.0.0/16 I tried a few other things, such as not using a CIDR and using a hostname to verify that there wasn't any obvious deviation in behavior introduced by the patch. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3998: res_pjsip: ami: Fix error in AMI output when no transport is associated to an endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3998/ --- (Updated Sept. 17, 2014, 5:43 p.m.) Review request for Asterisk Developers and Kevin Harwell. Changes --- Added ASTERISK-24161. Bugs: 24161 and 24331 https://issues.asterisk.org/jira/browse/24161 https://issues.asterisk.org/jira/browse/24331 Repository: Asterisk Description --- When no transport is associated to an endpoint, the AMI output for PJSIPShowEndpoint indicates an error instead of silently ignoring the missing transport. This patch causes the error to appear only if a transport was specified on the endpoint and the transport doesn't exist. It also fixes an issue with counting the objects that were actually found. Diffs - branches/12/res/res_pjsip_endpoint_identifier_ip.c 423274 branches/12/res/res_pjsip/pjsip_configuration.c 423274 branches/12/res/res_pjsip/location.c 423274 branches/12/res/res_pjsip/config_transport.c 423274 branches/12/res/res_pjsip/config_auth.c 423274 branches/12/include/asterisk/res_pjsip.h 423274 Diff: https://reviewboard.asterisk.org/r/3998/diff/ Testing --- Checked the output of PJSIPShowEndpoint to make sure the error appears (or doesn't) correctly and that ListItems is set correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3998: res_pjsip: ami: Fix error in AMI output when no transport is associated to an endpoint
On Sept. 17, 2014, 4:44 p.m., Mark Michelson wrote: I'd like to point out that the count fix will also solve issue ASTERISK-24161. Just as a note, when you commit this, the testsuite test channels/pjsip/ami/show_endpoint will start failing in Asterisk 13 and trunk since that test is adjusted to deal with the incorrect ListItems header reported back in the EndpointDetailComplete event. If you look in channels/pjsip/ami/show_endpoint/test-config.yaml and scroll all the way to the bottom, you'll see a XXX comment there explaining the problem. Right now, the test expects ListItems: '6', but that should be changed to ListItems: '7' after this patch goes in. This patch is only good for 12 since 13/trunk add the ContactStatusDetail event which will require a slight mod. I'll patch the testsuite when the 13/trunk patch goes up. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3998/#review13327 --- On Sept. 17, 2014, 5:43 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3998/ --- (Updated Sept. 17, 2014, 5:43 p.m.) Review request for Asterisk Developers and Kevin Harwell. Bugs: 24161 and 24331 https://issues.asterisk.org/jira/browse/24161 https://issues.asterisk.org/jira/browse/24331 Repository: Asterisk Description --- When no transport is associated to an endpoint, the AMI output for PJSIPShowEndpoint indicates an error instead of silently ignoring the missing transport. This patch causes the error to appear only if a transport was specified on the endpoint and the transport doesn't exist. It also fixes an issue with counting the objects that were actually found. Diffs - branches/12/res/res_pjsip_endpoint_identifier_ip.c 423274 branches/12/res/res_pjsip/pjsip_configuration.c 423274 branches/12/res/res_pjsip/location.c 423274 branches/12/res/res_pjsip/config_transport.c 423274 branches/12/res/res_pjsip/config_auth.c 423274 branches/12/include/asterisk/res_pjsip.h 423274 Diff: https://reviewboard.asterisk.org/r/3998/diff/ Testing --- Checked the output of PJSIPShowEndpoint to make sure the error appears (or doesn't) correctly and that ListItems is set correctly. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3993: config: bug: Fix SEGV in ast_category_insert when a matching category isn't found
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3993/ --- (Updated Sept. 18, 2014, 9:37 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423276 Repository: Asterisk Description --- If you call ast_category_insert with a match category that doesn't exist, the list traverse runs out of 'next' categories and you get a SEGV. This patch adds check for the end-of-list condition and changes the signature to return an int for success/failure indication instead of a void. The only consumer of this function is manager and it was also changed to use the return value. Diffs - branches/1.8/main/manager.c 423127 branches/1.8/main/config.c 423127 branches/1.8/include/asterisk/config.h 423127 Diff: https://reviewboard.asterisk.org/r/3993/diff/ Testing --- Tested with AMI UpdateConfig/newcat to make sure the proper value is returned in both positive and negative cases. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3989/
---
(Updated Sept. 18, 2014, 2:21 p.m.)
Status
--
This change has been marked as submitted.
Review request for Asterisk Developers and rmudgett.
Changes
---
Committed in revision 423476
Repository: Asterisk
Description
---
I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.
/*!
\brief Act like strsep but ignore separators inside quotes.
\param s Pointer to address of the the string to be processed.
Will be modified and can't be constant.
\param sep A single character delimiter.
\param flags Controls post-processing of the result.
AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
AST_STRSEP_STRIP does a trim then strips the outermost quotes. You may want
to trim again after the strip. Just OR both the TRIM and STRIP flags.
AST_STRSEP_UNESCAPE unescapes '\' sequences.
AST_STRSEP_ALL does all of the above processing.
\return The next token or NULL if done or if there are more than 8 levels of
nested quotes.
This function acts like strsep with three exceptions...
The separator is a single character instead of a string.
Separators inside quotes are treated literally instead of like separators.
You can elect to have leading and trailing whitespace and quotes
stripped from the result and have '\' sequences unescaped.
Like strsep, ast_strsep maintains no internal state and you can call it
recursively using different separators on the same storage.
Also like strsep, for consistent results, consecutive separators are not
collapsed so you may get an empty string as a valid result.
Examples:
\code
char *mystr = ast_strdupa(abc=def,ghi='zzz=yyy,456',jkl);
char *token, *token2, *token3;
while((token = ast_strsep(mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(token2, ',',
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}
\endcode
*/
char *ast_strsep(char **s, const char sep, uint32_t flags);
Diffs
-
branches/12/tests/test_strings.c 422963
branches/12/main/utils.c 422963
branches/12/include/asterisk/strings.h 422963
Diff: https://reviewboard.asterisk.org/r/3989/diff/
Testing
---
Thanks,
George Joseph
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 18, 2014, 2:42 p.m.) Review request for Asterisk Developers. Changes --- Updated summary Repository: Asterisk Description (updated) --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_register and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 422737 branches/12/main/chanvars.c 422737 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 422737 branches/12/configs/phoneprov.conf.sample 422737 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
/res_pjsip_phoneprov_provider.c PRE-CREATION branches/12/configs/pjsip.conf.sample 423501 Diff: https://reviewboard.asterisk.org/r/3976/diff/ Testing --- I'm already starting to convert sip peers to pjsip endpoints with no change to my Grandstream templates. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
On Sept. 18, 2014, 4:11 p.m., rmudgett wrote: branches/12/res/res_phoneprov.c, lines 128-129 https://reviewboard.asterisk.org/r/3970/diff/4/?file=67281#file67281line128 This is an interesting way of doing the standard ao2 container callback functions. Did you fix the formatting from the wiki page. Directly cutting and pasting the template function from the wiki gives you spaces instead of tabs. Yep, I reformatted it after pasting it. On Sept. 18, 2014, 4:11 p.m., rmudgett wrote: branches/12/res/res_phoneprov.c, line 983 https://reviewboard.asterisk.org/r/3970/diff/4/?file=67281#file67281line983 Is the cast necessary? Yes. The compiler complains note: expected ‘void *’ but argument is of type ‘ast_string_field’ - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/#review13348 --- On Sept. 18, 2014, 6:01 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 18, 2014, 6:01 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_register and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 423501 branches/12/main/chanvars.c 423501 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 423501 branches/12/configs/phoneprov.conf.sample 423501 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3970/ --- (Updated Sept. 18, 2014, 6:01 p.m.) Review request for Asterisk Developers. Changes --- Addressed Richard's comments. Repository: Asterisk Description --- The big piece missing for me to finally transition to pjsip was the ability to mirror the auto provisioning features of res_phoneprov. The first step (this patch) is to make res_phoneprov more modular so other modules (like pjsip) can provide configuration information instead of res_phoneprov relying solely on users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now exposed which allows config providers to register themselves, set defaults (server profile, etc) and add user extensions. ast_phoneprov_provider_register registers the provider and provides callbacks for loading default settings and loading users. ast_phoneprov_provider_unregister clears the defaults and users. ast_phoneprov_add_extension should be called once for each user/extension by the provider's load_users callback to add them. ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions deletes all extensions for the provider. res_phoneprov actually registers itself as the provider for sip/users and is always available and is the default. Writing a new provider... Since res_phoneprov is also it's own provider, examples of what a new provider would have to do are in load_users() in res_phoneprov.c. Those functions gather the information from users.conf and sip.conf and call the ast_provider_register and ast_phoneprov_add_extension apis. So... The provider creates a callback function which calls the ast_phoneprov_add_extension api for each user. It then calls ast_phoneprov_provider_register with the callback. res_phoneprov then calls the callback to cause the actual load. During normal http server ops, all work is done by res_phoneprov and the provider is never called again unless a reload is needed. If the provider wants to reload it can simply unregister and reregister or it can call its own load_users callback. If res_phoneprov wants to reload, it iterates over its registry and calls the providers callback. NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers were registered (other than itself) so a subsequent load will have nothing but it's own users. Additional changes... I added a few convenience functions to chanvars for creating lists and finding and deleting entries. No existing code was touched. Next steps... A provider for res_pjsip. Diffs (updated) - branches/12/res/res_phoneprov.exports.in PRE-CREATION branches/12/res/res_phoneprov.c 423501 branches/12/main/chanvars.c 423501 branches/12/include/asterisk/phoneprov.h PRE-CREATION branches/12/include/asterisk/chanvars.h 423501 branches/12/configs/phoneprov.conf.sample 423501 Diff: https://reviewboard.asterisk.org/r/3970/diff/ Testing --- I ran through several scenarios including the use of PP_EACH_USER and PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I actually use it with Grandstream phones and everything worked exactly as expected. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
On Sept. 18, 2014, 5:32 p.m., rmudgett wrote: branches/12/res/res_pjsip_phoneprov_provider.c, lines 71-75 https://reviewboard.asterisk.org/r/3976/diff/3/?file=67405#file67405line71 Concerned about the parameter names chosen. All uppercase parameter names is a bit unusual. I think there is a requirement for pjsip parameters to be snake_case for ARI. phoneprov variable have historically been upper case and they are case-sensitive. Having some upper and some lower would be not only confusing but it would make it impossible to use the same template file for both sip and pjsip providers. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/#review13359 --- On Sept. 18, 2014, 3:21 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 3:21 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a template substitution map as follows: ;MAC=deadbeef4dad ;added by pp1000 ;USERNAME=myname;automatically added by 1000-auth username ;SECRET=mysecret;automatically added by 1000-auth password ;PROFILE=someprofile;added by defaults ;SERVER=myserver.example.com;added by defaults ;SERVER_PORT=5060 ;added by defaults ;MYVAR=someUSERvalue;added by defaults but overdidden by user ;CALLERID=8005551212;automatically added by 1000 callerid ;DISPLAY_NAME=My Name ;automatically added by 1000
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
On Sept. 18, 2014, 5:32 p.m., rmudgett wrote: branches/12/res/res_pjsip_phoneprov_provider.c, line 174 https://reviewboard.asterisk.org/r/3976/diff/3/?file=67405#file67405line174 Another case where the if test burried in AST_VAR_LIST_INSERT_TAIL() is redundant. true but here I'm actually checking the return for the allocation failure and bailing. res_phoneprov always silently ignored the allocation failure and just didn't insert the variable into the list. Silently ignoring non-required variables can't hurt anything and if the system is so low on memory that it can't allocate a dozen bytes then you'll never see the error messages anyway. So, I guess I'll remove the check here and silently ignore. - George --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/#review13359 --- On Sept. 18, 2014, 3:21 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/ --- (Updated Sept. 18, 2014, 3:21 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This module allows res_pjsip to integrate with res_phoneprov and depends on the res_phoneprov refactor (r3970). A new pjsip.conf object is defined by this module... ;EXAMPLE PHONEPROV CONFIGURATION ; Before configuring provisioning here, see the documentation for res_phoneprov ; and configure phoneprov.conf appropriately. ; For each user to be autoprovisioned, a [phoneprov] configuration section ; must be created. At a minimum, the 'type', 'PROFILE' and 'MAC' variables must ; be set. All other variables are optional. ; Example: ;[1000] ;type=phoneprov ; must be specified as 'phoneprov' ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;PROFILE=digium ; required ;MAC=deadbeef4dad ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user confdigured variable ; If the phoneprov sections have common variables, it is best to create a ; phoneprov template. The example below will produce the same configuration ; as the one specified above except that MYVAR will be overridden for ; the specific user. ; Example: ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=digium ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someOTHERvalue ; A user confdigured variable ; To have USERNAME and SECRET automatically set, the endpoint ; specified here must in turn have an outbound_auth section defined. ; Fuller example: ;[1000] ;type=endpoint ;outbound_auth=1000-auth ;callerid=My Name 8005551212 ;transport=transport-udp-nat ;[1000-auth] ;type=auth ;auth_type=userpass ;username=myname ;password=mysecret ;[phoneprov_defaults](!) ;type=phoneprov ; must be specified as 'phoneprov' ;PROFILE=someprofile ; required ;SERVER=myserver.example.com ; A standard variable ;TIMEZONE=America/Denver ; A standard variable ;MYVAR=somevalue ; A user configured variable ;[1000](phoneprov_defaults) ;endpoint=1000; Required only if automatic setting of ; USERNAME, SECRET, DISPLAY_NAME and CALLERID ; are needed. ;MAC=deadbeef4dad ; required ;MYVAR=someUSERvalue ; A user confdigured variable ;LABEL=1000 ; A standard variable ; The previous sections would produce a template substitution map as follows: ;MAC=deadbeef4dad ;added by pp1000 ;USERNAME=myname;automatically added by 1000-auth username ;SECRET=mysecret;automatically added by 1000-auth password ;PROFILE=someprofile;added by defaults ;SERVER=myserver.example.com;added by defaults ;SERVER_PORT=5060 ;added by defaults ;MYVAR=someUSERvalue;added by defaults but overdidden by user ;CALLERID=8005551212
Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.
-CREATION branches/12/configs/pjsip.conf.sample 423501 Diff: https://reviewboard.asterisk.org/r/3976/diff/ Testing --- I'm already starting to convert sip peers to pjsip endpoints with no change to my Grandstream templates. Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
In the process of getting my GUI and real customers up on PJSIP I've come across a few things that could work better for me. One of them is the configuration process for ITSP trunks, specifically those that require registration and have more than 1 server. In a simple single-server scenario, a trunk would need 1 endpoint, 1 aor, 1 outbound_auth, 1 identify and 1 registration. Leaving out variables not needed for the example... [mytrunk] type = endpoint aors = mytrunk outbound_auth = mytrunk [mytrunk] type = aor contact = sip:sip1.myitsp.com [mytrunk] type = auth username = myuser password = mypass [mytrunk] type = identify endpoint = mytrunk match = sip1.myitsp.com [mytrunk] type = registration endpoint = mytrunk outbound_auth = mytrunk server_uri = sip:sip1.myitsp.com A little more verbose than chan_sip but pretty straightforward. I used the same name for all category/context/sections on purpose. It makes grouping everything that makes up the trunk a lot easier especially if you're using scripts or the AMI*(1)* to retrieve or modify the config. Now add a backup server... [mytrunk] type = endpoint aors = mytrunk outbound_auth = mytrunk [mytrunk] type = aor contact = sip:sip1.myitsp.com,sip:sip2.myitsp.com [mytrunk] type = auth username = myuser password = mypass [mytrunk] type = identify endpoint = mytrunk match = sip1.myitsp.com,sip2.myitsp.com [mytrunk-reg1] type = registration endpoint = mytrunk outbound_auth = mytrunk server_uri = sip:sip1.myitsp.com [mytrunk-reg2] type = registration endpoint = mytrunk outbound_auth = mytrunk server_uri = sip:sip2.myitsp.com I still have 1 endpoint, 1 aor, 1 auth, 1 identify but now I need 2 registrations because you can only have 1 server_uri in a registration, and they need special names so they don't conflict. The only thing different is the server_uri and just like contact and match, they're interchangeable in this scenario. My proposal is to allow registration/server_uri to be specified as a comma separated list or to be specified multiple times just like aor/contact and identify/match. This would allow us to manage only 1 registration section in the same manner as aor and identify. A registration would be Registered if at least 1 server was registered or I can add a registration_state_registered_at variable similar to device_state_busy_at which would specify the minimum number of servers needed to be considered Registered. If you actually want 2 registrations, nothing stops you from creating them. It seems like a minor issue but for me (and other folks I'm betting) the transition from chan_sip to chan_pjsip rests on getting tools, GUIs, scripts, etc. migrated. That variable number of registrations is a pain to deal with. Josh had some issues with this approach on IRC and suggested bringing the proposal to the list for wider discussion. What do you think? --- (1): Today you can't use AMI GetConfig/GetConfigJSON and UpdateConfig with a config file with multiple contexts with the same name. I'll have patches for that shortly which eventually will also allow res_sorcery_config to write back to its files. -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
On Sat, Sep 20, 2014 at 10:33 AM, Joshua Colp jc...@digium.com wrote: George Joseph wrote: snip My proposal is to allow registration/server_uri to be specified as a comma separated list or to be specified multiple times just like aor/contact and identify/match. This would allow us to manage only 1 registration section in the same manner as aor and identify. A registration would be Registered if at least 1 server was registered or I can add a registration_state_registered_at variable similar to device_state_busy_at which would specify the minimum number of servers needed to be considered Registered. If you actually want 2 registrations, nothing stops you from creating them. It seems like a minor issue but for me (and other folks I'm betting) the transition from chan_sip to chan_pjsip rests on getting tools, GUIs, scripts, etc. migrated. That variable number of registrations is a pain to deal with. Josh had some issues with this approach on IRC and suggested bringing the proposal to the list for wider discussion. I'll elaborate on why I dislike this: 1. It's overloading the outbound registration object to actually mean outbound registrations. This complicates the implementation and from a user perspective becomes harder to explain. Someone doesn't have to use it, but if they see it... they will... potentially without knowing what it means. 2. From a first glance and seeing the type as outbound registration I would expect that someone would think of it as an ordered failover list. If I can't register to the first then I'll try the second and so on. This is what some phones do. That's not what this would do. I take your point but PJSIP configuration is so much more complex than SIP configuration that there are lots of things that are going to trip people up. I don't think this one little point will matter. I've been in the code for a year and I'm still finding things I though worked one way but really worked another. 3. What it means to register to multiple URIs and how that should be interpreted is really environment specific. You've mentioned making what the cumulative result is configurable but knowing what failed and what succeeded individually is still important. It may not be for some ITSPs, it may be for others. If addressed individually you get this information already and it can be interpreted. The CLI and AMI could show individual server_uri status just like they do individual contact status in an aor. 4. I'm hesitant to push this into the outbound registration module as the solution. I'm all for making things easier but having these lower level blocks as simple and direct as possible is valuable. Trying not to cross the line is hard. Agreed. 5. The idea of higher level concept configuration has been thrown around as something to make this easier. I personally think this sort of thing belongs there. A type=trunk, itsp, phone, etc. Lower level blocks remain the same and additional logic on top can be added to handle this sort of thing. Are you thinking like users.conf? I thought you guys wanted that to die a horrible death. :) Seriously though, are you thinking along the lines of a new composite pjsip configuration object that creates the base objects behind the scenes? If so, that'd solve a lot and I could start working on it right now. I just thought you guys were shying away from these types of things. I look forward to seeing what others think! Me too! Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
On Sat, Sep 20, 2014 at 11:21 AM, Joshua Colp jc...@digium.com wrote: George Joseph wrote: snip 5. The idea of higher level concept configuration has been thrown around as something to make this easier. I personally think this sort of thing belongs there. A type=trunk, itsp, phone, etc. Lower level blocks remain the same and additional logic on top can be added to handle this sort of thing. Are you thinking like users.conf? I thought you guys wanted that to die a horrible death. :) Seriously though, are you thinking along the lines of a new composite pjsip configuration object that creates the base objects behind the scenes? If so, that'd solve a lot and I could start working on it right now. I just thought you guys were shying away from these types of things. As base objects it's a bad idea. As a single object to rule them all (a user) it's also a complicated/bad idea. As higher level concepts which represent things that people are familiar with they're fine. Since endpoint really contains most of the detailed config parameters, would you see enhancements to endpoint that allow direct specification of simple things like username, password, contact, etc. or really a separate object like trunk, user, etc.? I'm guessing the latter but the former would be a lot easier to implement. -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
On Sat, Sep 20, 2014 at 12:06 PM, George Joseph george.jos...@fairview5.com wrote: On Sat, Sep 20, 2014 at 11:21 AM, Joshua Colp jc...@digium.com wrote: George Joseph wrote: snip 5. The idea of higher level concept configuration has been thrown around as something to make this easier. I personally think this sort of thing belongs there. A type=trunk, itsp, phone, etc. Lower level blocks remain the same and additional logic on top can be added to handle this sort of thing. Are you thinking like users.conf? I thought you guys wanted that to die a horrible death. :) Seriously though, are you thinking along the lines of a new composite pjsip configuration object that creates the base objects behind the scenes? If so, that'd solve a lot and I could start working on it right now. I just thought you guys were shying away from these types of things. As base objects it's a bad idea. As a single object to rule them all (a user) it's also a complicated/bad idea. As higher level concepts which represent things that people are familiar with they're fine. Since endpoint really contains most of the detailed config parameters, would you see enhancements to endpoint that allow direct specification of simple things like username, password, contact, etc. or really a separate object like trunk, user, etc.? I'm guessing the latter but the former would be a lot easier to implement. Or separate objects from a config file perspective but implemented in pjsip_configuration with endpoint. -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
On Sat, Sep 20, 2014 at 1:10 PM, Joshua Colp jc...@digium.com wrote: George Joseph wrote: snip Or separate objects from a config file perspective but implemented in pjsip_configuration with endpoint. Completely separate. Mixing the two defeats the purpose of having a clear boundary. Ok, how about this... 2 new object types called composite and pattern (or whatever) implemented in a separate res_pjsip_* module [mytrunk] type = composite pattern = trunk etc... [trunk] type = pattern register = yes contacts = static outbound_auth = yes inbound_auth = no identify = yes -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
On Sat, Sep 20, 2014 at 1:35 PM, Joshua Colp jc...@digium.com wrote: George Joseph wrote: On Sat, Sep 20, 2014 at 1:10 PM, Joshua Colp jc...@digium.com mailto:jc...@digium.com wrote: George Joseph wrote: snip Or separate objects from a config file perspective but implemented in pjsip_configuration with endpoint. Completely separate. Mixing the two defeats the purpose of having a clear boundary. Ok, how about this... 2 new object types called composite and pattern (or whatever) implemented in a separate res_pjsip_* module [mytrunk] type = composite pattern = trunk etc... [trunk] type = pattern register = yes contacts = static outbound_auth = yes inbound_auth = no identify = yes I don't understand the naming or what they mean at first glance ^_^ I was thinking that we probably don't want to create hard coded objects called trunk, user, etc. Instead let the user define the patterns that suit them. So, define a pattern first... [trunk-pattern] type = pattern contacts = static ; would cause a contact line to appear in the aor ; for each server register = yes ; would cause a registration object to be created for each ; server outbound_auth = yes ; would cause an auth object to be created which the ; endpoint would reference as outbound_auth inbound_auth = no ; would do the same thing for endpoint/auth identify = yes ; would cause an identify object to be created with a ; match for each server Actually, the pattern could specify how to construct other variables... client_uri = sip:%OU@%S contact = sip:%S server_uri = sip:%S Now for each trunk... [mytrunk] type = composite pattern = trunk-pattern server = sip1.itsp.com server = sip2.itsp.com outbound_username = myusername outbound_password = mypassword would be the equivalent of... [mytrunk] type = endpoint aors = mytrunk outbound_auth = mytrunk-auth [mytrunk] type = aor contact = sip:sip1.itsp.com contact = sip:sip2.itsp.com [mytrunk-auth] type = auth username = myusername password = mypassword [mytrunk-id] type = identify endpoint = mytrunk match = sip1.itsp.com match = sip2.itsp.com [myrunk-reg-1] type = registration endpoint = mytrunk client_uri = sip:myusern...@sip1.itsp.com server_uri = sip:sip1.itsp.com [myrunk-reg-2] type = registration endpoint = mytrunk client_uri = sip:myusern...@sip2.itsp.com server_uri = sip:sip2.itsp.com -- _ -- 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
