Summary:	ASTERISK-05929: [patch] cleanup ast_dtmf_stream code and documentation
Reporter:	Russell Bryant (russell)	Labels:
Date Opened:	2005-12-29 14:35:51.000-0600	Date Closed:	2008-01-15 16:16:24.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) ast_dtmf_stream.rev2.before_after ( 1) ast_dtmf_stream.rev2.patch
Description:	The attached patch addresses some minor issues in the function ast_dtmf_stream: 1) The middle of the function is a for loop that iterates through the dtmf string and writes the appropriate DTMF frames to the channel.  Inside of this loop, the frame was being cleared using memset() and all of the members reset.  2 of the 3 members being used in the frame are always the same.  I set these 2 fields to this value at the beginning of the function, removed the memset, and only reset the frame subclass inside the loop. 2) If a peer channel is provided, it will be autoserviced while the primary channel is receiving dtmf.  However, if any error were to occur and 'res' is set to a non-zero value, this value could be lost and 'res' could be set back to zero when we stop the autoservice on the peer channel.  I have made it so we will no longer lose the non-zero result if it is set before stopping the autoservice on the peer channel. 3) I have added doxygen style documentation for this function. 4) The rest of the changes are to clean up the formatting of the function. ****** ADDITIONAL INFORMATION ****** I am going to try to stop directly commiting most of my code cleanup patches.  I figure that it is probably best to have someone at least take a quick look at my changes before they go in.
Comments:	By: Luigi Rizzo (rizzo) 2005-12-30 17:01:52.000-0600 the code sounds good. A minor improvement could be to change the body of the for loop as follows:  if (*ptr == 'w') { /* w = wait 500ms */      ...  } else if (strchr(...)) { /* valid dtmf */      f.subclass = *ptr;      ...  } else      ast_log(...); so you don't need to put a continue; statement in the middle, and also, change digits and ptr to "const char *" since the string is not modified. By: Russell Bryant (russell) 2005-12-31 00:09:09.000-0600 Thanks for taking a look at the patch!   I have updated the patch to reflect the provided suggestions. By: Digium Subversion (svnbot) 2008-01-15 16:14:06.000-0600 Repository: asterisk Revision: 7969 U   trunk/app.c U   trunk/include/asterisk/app.h ------------------------------------------------------------------------ r7969 | russell | 2008-01-15 16:14:05 -0600 (Tue, 15 Jan 2008) | 3 lines add doxygen documentation and fix various issues with ast_dtmf_stream (discussed in issue ASTERISK-5929) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=7969 By: Digium Subversion (svnbot) 2008-01-15 16:14:07.000-0600 Repository: asterisk Revision: 7970 U   branches/1.2/app.c ------------------------------------------------------------------------ r7970 | russell | 2008-01-15 16:14:06 -0600 (Tue, 15 Jan 2008) | 3 lines don't override an error condition that occurred when acting on the primary channel when stopping the autoservice on the peer channel.  (from issue ASTERISK-5929) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=7970 By: Digium Subversion (svnbot) 2008-01-15 16:15:20.000-0600 Repository: asterisk Revision: 8052 _U  team/crichter/0.2.1/ U   team/crichter/0.2.1/app.c U   team/crichter/0.2.1/apps/app_dial.c U   team/crichter/0.2.1/apps/app_voicemail.c U   team/crichter/0.2.1/channel.c U   team/crichter/0.2.1/channels/chan_agent.c U   team/crichter/0.2.1/channels/chan_sip.c U   team/crichter/0.2.1/configs/voicemail.conf.sample U   team/crichter/0.2.1/pbx.c U   team/crichter/0.2.1/translate.c ------------------------------------------------------------------------ r8052 | crichter | 2008-01-15 16:15:20 -0600 (Tue, 15 Jan 2008) | 61 lines Merged revisions 7955,7957,7960,7963,7965,7970,7972,7976,7986,7999,8047 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r7955 | tilghman | 2006-01-11 02:30:10 +0100 (Mi, 11 Jan 2006) | 2 lines Bug 6192 - behave correctly when mailbox is specified as argument ........ r7957 | russell | 2006-01-11 04:12:44 +0100 (Mi, 11 Jan 2006) | 2 lines fix a little typo ........ r7960 | russell | 2006-01-11 05:19:21 +0100 (Mi, 11 Jan 2006) | 2 lines fix locking error - lock instead of unlock ........ r7963 | mogorman | 2006-01-11 05:38:07 +0100 (Mi, 11 Jan 2006) | 2 lines Minor typo refrenced in 6191 ........ r7965 | russell | 2006-01-11 05:53:24 +0100 (Mi, 11 Jan 2006) | 2 lines lock list of translators *before* recalculating the translation matrix ........ r7970 | russell | 2006-01-11 06:26:21 +0100 (Mi, 11 Jan 2006) | 3 lines don't override an error condition that occurred when acting on the primary channel when stopping the autoservice on the peer channel.  (from issue ASTERISK-5929) ........ r7972 | russell | 2006-01-11 06:46:39 +0100 (Mi, 11 Jan 2006) | 2 lines fix mem leak on module unload (issue ASTERISK-6033) ........ r7976 | russell | 2006-01-11 08:18:16 +0100 (Mi, 11 Jan 2006) | 2 lines fix temp greetings with ODBC storage (issue ASTERISK-5920) ........ r7986 | russell | 2006-01-11 20:08:53 +0100 (Mi, 11 Jan 2006) | 2 lines move variable to correct scope (issue ASTERISK-6040) ........ r7999 | tilghman | 2006-01-12 07:14:22 +0100 (Do, 12 Jan 2006) | 2 lines Bug 6211 - Add option deletevoicemail as equivalent to option delete for Realtime ........ r8047 | russell | 2006-01-13 07:07:39 +0100 (Fr, 13 Jan 2006) | 2 lines fix spelling errors (issue ASTERISK-6069) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=8052 By: Digium Subversion (svnbot) 2008-01-15 16:16:13.000-0600 Repository: asterisk Revision: 8109 _U  team/oej/metermaids/ U   team/oej/metermaids/app.c U   team/oej/metermaids/apps/app_dial.c U   team/oej/metermaids/apps/app_voicemail.c U   team/oej/metermaids/channel.c U   team/oej/metermaids/channels/chan_agent.c U   team/oej/metermaids/channels/chan_iax2.c U   team/oej/metermaids/channels/chan_sip.c U   team/oej/metermaids/configs/voicemail.conf.sample U   team/oej/metermaids/doc/README.cdr U   team/oej/metermaids/file.c U   team/oej/metermaids/funcs/func_strings.c U   team/oej/metermaids/pbx.c U   team/oej/metermaids/translate.c ------------------------------------------------------------------------ r8109 | oej | 2008-01-15 16:16:13 -0600 (Tue, 15 Jan 2008) | 82 lines Merged revisions 7915,7917,7939,7955,7957,7960,7963,7965,7970,7972,7976,7986,7999,8047,8074 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r7915 | russell | 2006-01-09 23:07:26 +0100 (Mon, 09 Jan 2006) | 2 lines add missing unlock (issue ASTERISK-5954) ........ r7917 | kpfleming | 2006-01-09 23:48:48 +0100 (Mon, 09 Jan 2006) | 2 lines re-initialize _all_ sequence numbers when transfer completes ........ r7939 | oej | 2006-01-10 09:48:14 +0100 (Tue, 10 Jan 2006) | 3 lines - Adding reference to README.tds - Reformatting table ........ r7955 | tilghman | 2006-01-11 02:30:10 +0100 (Wed, 11 Jan 2006) | 2 lines Bug 6192 - behave correctly when mailbox is specified as argument ........ r7957 | russell | 2006-01-11 04:12:44 +0100 (Wed, 11 Jan 2006) | 2 lines fix a little typo ........ r7960 | russell | 2006-01-11 05:19:21 +0100 (Wed, 11 Jan 2006) | 2 lines fix locking error - lock instead of unlock ........ r7963 | mogorman | 2006-01-11 05:38:07 +0100 (Wed, 11 Jan 2006) | 2 lines Minor typo refrenced in 6191 ........ r7965 | russell | 2006-01-11 05:53:24 +0100 (Wed, 11 Jan 2006) | 2 lines lock list of translators *before* recalculating the translation matrix ........ r7970 | russell | 2006-01-11 06:26:21 +0100 (Wed, 11 Jan 2006) | 3 lines don't override an error condition that occurred when acting on the primary channel when stopping the autoservice on the peer channel.  (from issue ASTERISK-5929) ........ r7972 | russell | 2006-01-11 06:46:39 +0100 (Wed, 11 Jan 2006) | 2 lines fix mem leak on module unload (issue ASTERISK-6033) ........ r7976 | russell | 2006-01-11 08:18:16 +0100 (Wed, 11 Jan 2006) | 2 lines fix temp greetings with ODBC storage (issue ASTERISK-5920) ........ r7986 | russell | 2006-01-11 20:08:53 +0100 (Wed, 11 Jan 2006) | 2 lines move variable to correct scope (issue ASTERISK-6040) ........ r7999 | tilghman | 2006-01-12 07:14:22 +0100 (Thu, 12 Jan 2006) | 2 lines Bug 6211 - Add option deletevoicemail as equivalent to option delete for Realtime ........ r8047 | russell | 2006-01-13 07:07:39 +0100 (Fri, 13 Jan 2006) | 2 lines fix spelling errors (issue ASTERISK-6069) ........ r8074 | tilghman | 2006-01-14 20:06:44 +0100 (Sat, 14 Jan 2006) | 2 lines Bug 6238 - Fix segfault when delimiter not specified ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=8109 By: Digium Subversion (svnbot) 2008-01-15 16:16:15.000-0600 Repository: asterisk Revision: 8110 _U  team/oej/managerstuff/ U   team/oej/managerstuff/app.c U   team/oej/managerstuff/apps/app_dial.c U   team/oej/managerstuff/apps/app_voicemail.c U   team/oej/managerstuff/channel.c U   team/oej/managerstuff/channels/chan_agent.c U   team/oej/managerstuff/channels/chan_sip.c U   team/oej/managerstuff/configs/voicemail.conf.sample U   team/oej/managerstuff/doc/README.cdr U   team/oej/managerstuff/funcs/func_strings.c U   team/oej/managerstuff/pbx.c U   team/oej/managerstuff/translate.c ------------------------------------------------------------------------ r8110 | oej | 2008-01-15 16:16:15 -0600 (Tue, 15 Jan 2008) | 72 lines Merged revisions 7939,7955,7957,7960,7963,7965,7970,7972,7976,7986,7999,8047,8074 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r7939 | oej | 2006-01-10 09:48:14 +0100 (Tue, 10 Jan 2006) | 3 lines - Adding reference to README.tds - Reformatting table ........ r7955 | tilghman | 2006-01-11 02:30:10 +0100 (Wed, 11 Jan 2006) | 2 lines Bug 6192 - behave correctly when mailbox is specified as argument ........ r7957 | russell | 2006-01-11 04:12:44 +0100 (Wed, 11 Jan 2006) | 2 lines fix a little typo ........ r7960 | russell | 2006-01-11 05:19:21 +0100 (Wed, 11 Jan 2006) | 2 lines fix locking error - lock instead of unlock ........ r7963 | mogorman | 2006-01-11 05:38:07 +0100 (Wed, 11 Jan 2006) | 2 lines Minor typo refrenced in 6191 ........ r7965 | russell | 2006-01-11 05:53:24 +0100 (Wed, 11 Jan 2006) | 2 lines lock list of translators *before* recalculating the translation matrix ........ r7970 | russell | 2006-01-11 06:26:21 +0100 (Wed, 11 Jan 2006) | 3 lines don't override an error condition that occurred when acting on the primary channel when stopping the autoservice on the peer channel.  (from issue ASTERISK-5929) ........ r7972 | russell | 2006-01-11 06:46:39 +0100 (Wed, 11 Jan 2006) | 2 lines fix mem leak on module unload (issue ASTERISK-6033) ........ r7976 | russell | 2006-01-11 08:18:16 +0100 (Wed, 11 Jan 2006) | 2 lines fix temp greetings with ODBC storage (issue ASTERISK-5920) ........ r7986 | russell | 2006-01-11 20:08:53 +0100 (Wed, 11 Jan 2006) | 2 lines move variable to correct scope (issue ASTERISK-6040) ........ r7999 | tilghman | 2006-01-12 07:14:22 +0100 (Thu, 12 Jan 2006) | 2 lines Bug 6211 - Add option deletevoicemail as equivalent to option delete for Realtime ........ r8047 | russell | 2006-01-13 07:07:39 +0100 (Fri, 13 Jan 2006) | 2 lines fix spelling errors (issue ASTERISK-6069) ........ r8074 | tilghman | 2006-01-14 20:06:44 +0100 (Sat, 14 Jan 2006) | 2 lines Bug 6238 - Fix segfault when delimiter not specified ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=8110 By: Digium Subversion (svnbot) 2008-01-15 16:16:24.000-0600 Repository: asterisk Revision: 8119 _U  team/oej/moduletest/ U   team/oej/moduletest/app.c U   team/oej/moduletest/apps/app_dial.c U   team/oej/moduletest/apps/app_voicemail.c U   team/oej/moduletest/asterisk.c U   team/oej/moduletest/channel.c U   team/oej/moduletest/channels/chan_agent.c U   team/oej/moduletest/channels/chan_iax2.c U   team/oej/moduletest/channels/chan_sip.c U   team/oej/moduletest/configs/voicemail.conf.sample U   team/oej/moduletest/db.c U   team/oej/moduletest/doc/README.cdr U   team/oej/moduletest/doc/README.variables U   team/oej/moduletest/file.c U   team/oej/moduletest/funcs/func_strings.c U   team/oej/moduletest/pbx/pbx_spool.c U   team/oej/moduletest/pbx.c U   team/oej/moduletest/translate.c ------------------------------------------------------------------------ r8119 | oej | 2008-01-15 16:16:24 -0600 (Tue, 15 Jan 2008) | 112 lines Merged revisions 7898-7900,7904,7908,7915,7917,7939,7955,7957,7960,7963,7965,7970,7972,7976,7986,7999,8047,8074,8112 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.2 ........ r7898 | kpfleming | 2006-01-09 19:08:07 +0100 (Mon, 09 Jan 2006) | 2 lines fix breakage introduced in revision 7871 ........ r7899 | kpfleming | 2006-01-09 19:09:53 +0100 (Mon, 09 Jan 2006) | 2 lines backport fix from revision 7856 of trunk ........ r7900 | kpfleming | 2006-01-09 19:11:23 +0100 (Mon, 09 Jan 2006) | 2 lines commit user/group-related changes from trunk ........ r7904 | tilghman | 2006-01-09 19:37:50 +0100 (Mon, 09 Jan 2006) | 2 lines Update variable documentation to match the code ........ r7908 | tilghman | 2006-01-09 21:08:24 +0100 (Mon, 09 Jan 2006) | 2 lines Bug 6157 - Memory leak ........ r7915 | russell | 2006-01-09 23:07:26 +0100 (Mon, 09 Jan 2006) | 2 lines add missing unlock (issue ASTERISK-5954) ........ r7917 | kpfleming | 2006-01-09 23:48:48 +0100 (Mon, 09 Jan 2006) | 2 lines re-initialize _all_ sequence numbers when transfer completes ........ r7939 | oej | 2006-01-10 09:48:14 +0100 (Tue, 10 Jan 2006) | 3 lines - Adding reference to README.tds - Reformatting table ........ r7955 | tilghman | 2006-01-11 02:30:10 +0100 (Wed, 11 Jan 2006) | 2 lines Bug 6192 - behave correctly when mailbox is specified as argument ........ r7957 | russell | 2006-01-11 04:12:44 +0100 (Wed, 11 Jan 2006) | 2 lines fix a little typo ........ r7960 | russell | 2006-01-11 05:19:21 +0100 (Wed, 11 Jan 2006) | 2 lines fix locking error - lock instead of unlock ........ r7963 | mogorman | 2006-01-11 05:38:07 +0100 (Wed, 11 Jan 2006) | 2 lines Minor typo refrenced in 6191 ........ r7965 | russell | 2006-01-11 05:53:24 +0100 (Wed, 11 Jan 2006) | 2 lines lock list of translators *before* recalculating the translation matrix ........ r7970 | russell | 2006-01-11 06:26:21 +0100 (Wed, 11 Jan 2006) | 3 lines don't override an error condition that occurred when acting on the primary channel when stopping the autoservice on the peer channel.  (from issue ASTERISK-5929) ........ r7972 | russell | 2006-01-11 06:46:39 +0100 (Wed, 11 Jan 2006) | 2 lines fix mem leak on module unload (issue ASTERISK-6033) ........ r7976 | russell | 2006-01-11 08:18:16 +0100 (Wed, 11 Jan 2006) | 2 lines fix temp greetings with ODBC storage (issue ASTERISK-5920) ........ r7986 | russell | 2006-01-11 20:08:53 +0100 (Wed, 11 Jan 2006) | 2 lines move variable to correct scope (issue ASTERISK-6040) ........ r7999 | tilghman | 2006-01-12 07:14:22 +0100 (Thu, 12 Jan 2006) | 2 lines Bug 6211 - Add option deletevoicemail as equivalent to option delete for Realtime ........ r8047 | russell | 2006-01-13 07:07:39 +0100 (Fri, 13 Jan 2006) | 2 lines fix spelling errors (issue ASTERISK-6069) ........ r8074 | tilghman | 2006-01-14 20:06:44 +0100 (Sat, 14 Jan 2006) | 2 lines Bug 6238 - Fix segfault when delimiter not specified ........ r8112 | kpfleming | 2006-01-17 00:51:37 +0100 (Tue, 17 Jan 2006) | 2 lines do rlimit check _after_ reading config file, in case 'dumpcore' is specified there ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=8119