Summary: | ASTERISK-10209: various coding errors causing more trouble than reasonable | ||
Reporter: | Jonathan Towne (jontow) | Labels: | |
Date Opened: | 2007-08-30 20:43:29 | Date Closed: | 2007-11-02 10:22:35 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | CDR/cdr_odbc |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) 20070830__issue10614.diff.txt | |
Description: | Avoid SQLDisconnect() anywhere but odbc_disconnect() (this means in odbc_log()). Get the query prepare functions out of odbc_log(), they don't belong there and it doesn't work: on failure, it'll never succeed because the connection is trashed and things are never re-prepared after being allocated. In all of the file, ODBC_res should be of type SQLRETURN, not a normal int. In odbc_do_query(), proper error reporting from the ODBC layer doesn't hurt either; it doesn't crash the drivers if you don't hard-code sizes that are way too small per the specs and use the correct SQL* types. No, I don't have a patch that I can post right now, and no, I don't have a valid disclaimer these days. | ||
Comments: | By: Tilghman Lesher (tilghman) 2007-08-30 22:55:27 Do you have a specific crash that this is causing? By: Tilghman Lesher (tilghman) 2007-08-30 23:47:49 Really, the only way to go with this is to convert the module over to use res_odbc connection management. This also removes the single-threadedness that has characterized this module since it was written. I would encourage you to look at cdr_adaptive_odbc, as it is far more flexible. I daresay cdr_odbc will be replaced with cdr_adaptive_odbc after the next release cycle. By: Digium Subversion (svnbot) 2007-11-01 17:42:28 Repository: asterisk Revision: 88182 U trunk/UPGRADE.txt U trunk/cdr/cdr_odbc.c ------------------------------------------------------------------------ r88182 | tilghman | 2007-11-01 17:42:27 -0500 (Thu, 01 Nov 2007) | 3 lines Convert cdr_odbc to use res_odbc managed connections Closes issue ASTERISK-10209 ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-11-02 09:38:03 Repository: asterisk Revision: 88245 _U team/murf/fast-ast2/ U team/murf/fast-ast2/UPGRADE.txt U team/murf/fast-ast2/apps/app_dial.c U team/murf/fast-ast2/apps/app_exec.c U team/murf/fast-ast2/apps/app_macro.c U team/murf/fast-ast2/apps/app_minivm.c U team/murf/fast-ast2/apps/app_mixmonitor.c U team/murf/fast-ast2/apps/app_playback.c U team/murf/fast-ast2/apps/app_queue.c U team/murf/fast-ast2/apps/app_rpt.c U team/murf/fast-ast2/cdr/cdr_custom.c U team/murf/fast-ast2/cdr/cdr_manager.c U team/murf/fast-ast2/cdr/cdr_odbc.c U team/murf/fast-ast2/cdr/cdr_sqlite3_custom.c U team/murf/fast-ast2/channels/chan_gtalk.c U team/murf/fast-ast2/channels/chan_jingle.c U team/murf/fast-ast2/configure U team/murf/fast-ast2/configure.ac U team/murf/fast-ast2/funcs/func_cut.c U team/murf/fast-ast2/funcs/func_logic.c U team/murf/fast-ast2/funcs/func_odbc.c U team/murf/fast-ast2/funcs/func_strings.c U team/murf/fast-ast2/include/asterisk/autoconfig.h.in U team/murf/fast-ast2/include/asterisk/jabber.h U team/murf/fast-ast2/include/asterisk/pbx.h U team/murf/fast-ast2/main/logger.c U team/murf/fast-ast2/main/pbx.c U team/murf/fast-ast2/makeopts.in U team/murf/fast-ast2/pbx/pbx_config.c U team/murf/fast-ast2/pbx/pbx_dundi.c U team/murf/fast-ast2/pbx/pbx_loopback.c U team/murf/fast-ast2/pbx/pbx_realtime.c U team/murf/fast-ast2/res/ael/pval.c U team/murf/fast-ast2/res/res_agi.c U team/murf/fast-ast2/res/res_jabber.c U team/murf/fast-ast2/utils/extconf.c ------------------------------------------------------------------------ r88245 | murf | 2007-11-02 09:38:02 -0500 (Fri, 02 Nov 2007) | 39 lines Merged revisions 88164-88166,88182-88184 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r88164 | qwell | 2007-11-01 16:10:33 -0600 (Thu, 01 Nov 2007) | 4 lines Switch res_jabber to use openssl rather than gnutls. Closes issue ASTERISK-9675, patch by phsultan. Copied from branch at http://svn.digium.com/svn/asterisk/team/phsultan/res_jabber-openssl/ ........ r88165 | qwell | 2007-11-01 16:19:56 -0600 (Thu, 01 Nov 2007) | 1 line Crap, accidentally copied the props. Thanks for pointing this out mvanbaak. The odds are quite high that this will break automerge on every team branch. ........ r88166 | murf | 2007-11-01 16:26:51 -0600 (Thu, 01 Nov 2007) | 1 line This commits the performance mods that give the priority processing engine in the pbx, a 25-30peed boost. The two updates used, are, first, to merge the ast_exists_extension() and the ast_spawn_extension() where they are called sequentially in a loop in the code, into a slightly upgraded version of ast_spawn_extension(), with a few extra args; and, second, I modified the substitute_variables_helper_full, so it zeroes out the byte after the evaluated string instead of demanding you pre-zero the buffer; I also went thru the code and removed the code that zeroed this buffer before every call to the substitute_variables_helper_full. The first fix provides about a 9peedup, and the second the rest. These figures come from the 'PIPS' benchmark I describe in blogs, conf. reports, etc. ........ r88182 | tilghman | 2007-11-01 16:43:46 -0600 (Thu, 01 Nov 2007) | 3 lines Convert cdr_odbc to use res_odbc managed connections Closes issue ASTERISK-10209 ........ r88183 | tilghman | 2007-11-01 17:26:35 -0600 (Thu, 01 Nov 2007) | 3 lines Modify WaitExten to include an optional dialtone Closes issue ASTERISK-10356 ........ r88184 | qwell | 2007-11-01 17:26:51 -0600 (Thu, 01 Nov 2007) | 1 line Remove traces of gnutls, since we no longer use/need it. ........ Made some changes to the pbx.c stuff, to duplicate flow of control better in the merged exists_extension/spawn_extension calls. ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-11-02 10:22:35 Repository: asterisk Revision: 88246 _U team/group/CDRfix5/ U team/group/CDRfix5/UPGRADE.txt U team/group/CDRfix5/apps/app_dial.c U team/group/CDRfix5/apps/app_exec.c U team/group/CDRfix5/apps/app_macro.c U team/group/CDRfix5/apps/app_minivm.c U team/group/CDRfix5/apps/app_mixmonitor.c U team/group/CDRfix5/apps/app_playback.c U team/group/CDRfix5/apps/app_queue.c U team/group/CDRfix5/apps/app_rpt.c U team/group/CDRfix5/cdr/cdr_custom.c U team/group/CDRfix5/cdr/cdr_manager.c U team/group/CDRfix5/cdr/cdr_odbc.c U team/group/CDRfix5/cdr/cdr_sqlite3_custom.c U team/group/CDRfix5/channels/chan_gtalk.c U team/group/CDRfix5/channels/chan_jingle.c U team/group/CDRfix5/configure U team/group/CDRfix5/configure.ac U team/group/CDRfix5/funcs/func_cut.c U team/group/CDRfix5/funcs/func_logic.c U team/group/CDRfix5/funcs/func_odbc.c U team/group/CDRfix5/funcs/func_strings.c U team/group/CDRfix5/include/asterisk/autoconfig.h.in U team/group/CDRfix5/include/asterisk/jabber.h U team/group/CDRfix5/include/asterisk/lock.h U team/group/CDRfix5/include/asterisk/pbx.h U team/group/CDRfix5/main/config.c U team/group/CDRfix5/main/logger.c U team/group/CDRfix5/main/pbx.c U team/group/CDRfix5/makeopts.in U team/group/CDRfix5/pbx/pbx_config.c U team/group/CDRfix5/pbx/pbx_dundi.c U team/group/CDRfix5/pbx/pbx_loopback.c U team/group/CDRfix5/pbx/pbx_realtime.c U team/group/CDRfix5/res/ael/pval.c U team/group/CDRfix5/res/res_agi.c U team/group/CDRfix5/res/res_jabber.c U team/group/CDRfix5/utils/extconf.c ------------------------------------------------------------------------ r88246 | murf | 2007-11-02 10:22:34 -0500 (Fri, 02 Nov 2007) | 73 lines Merged revisions 88164-88166,88182-88184,88197,88209,88211-88212 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ................ r88164 | qwell | 2007-11-01 16:10:33 -0600 (Thu, 01 Nov 2007) | 4 lines Switch res_jabber to use openssl rather than gnutls. Closes issue ASTERISK-9675, patch by phsultan. Copied from branch at http://svn.digium.com/svn/asterisk/team/phsultan/res_jabber-openssl/ ................ r88165 | qwell | 2007-11-01 16:19:56 -0600 (Thu, 01 Nov 2007) | 1 line Crap, accidentally copied the props. Thanks for pointing this out mvanbaak. The odds are quite high that this will break automerge on every team branch. ................ r88166 | murf | 2007-11-01 16:26:51 -0600 (Thu, 01 Nov 2007) | 1 line This commits the performance mods that give the priority processing engine in the pbx, a 25-30peed boost. The two updates used, are, first, to merge the ast_exists_extension() and the ast_spawn_extension() where they are called sequentially in a loop in the code, into a slightly upgraded version of ast_spawn_extension(), with a few extra args; and, second, I modified the substitute_variables_helper_full, so it zeroes out the byte after the evaluated string instead of demanding you pre-zero the buffer; I also went thru the code and removed the code that zeroed this buffer before every call to the substitute_variables_helper_full. The first fix provides about a 9peedup, and the second the rest. These figures come from the 'PIPS' benchmark I describe in blogs, conf. reports, etc. ................ r88182 | tilghman | 2007-11-01 16:43:46 -0600 (Thu, 01 Nov 2007) | 3 lines Convert cdr_odbc to use res_odbc managed connections Closes issue ASTERISK-10209 ................ r88183 | tilghman | 2007-11-01 17:26:35 -0600 (Thu, 01 Nov 2007) | 3 lines Modify WaitExten to include an optional dialtone Closes issue ASTERISK-10356 ................ r88184 | qwell | 2007-11-01 17:26:51 -0600 (Thu, 01 Nov 2007) | 1 line Remove traces of gnutls, since we no longer use/need it. ................ r88197 | file | 2007-11-01 21:09:02 -0600 (Thu, 01 Nov 2007) | 2 lines Restore building under 64-bit platforms. ................ r88209 | tilghman | 2007-11-02 06:54:31 -0600 (Fri, 02 Nov 2007) | 5 lines 'h' extension doesn't execute past first priority Reported by: dimas Patch by: dimas Closes bug ASTERISK-10672 ................ r88211 | tilghman | 2007-11-02 07:10:29 -0600 (Fri, 02 Nov 2007) | 13 lines Merged revisions 88210 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r88210 | tilghman | 2007-11-02 08:03:03 -0500 (Fri, 02 Nov 2007) | 5 lines Fix build on Solaris Reported by: snuffy Patch by: ys Closes issue ASTERISK-10669 ........ ................ r88212 | tilghman | 2007-11-02 07:17:48 -0600 (Fri, 02 Nov 2007) | 5 lines Don't re-cache the filename, but check to see if it already exists Reported by: jamesgolovich Patch by: jamesgolovich Closes issue ASTERISK-10670 ................ ------------------------------------------------------------------------ |