Summary:	ASTERISK-05707: [patch][post 1.4] app_dial code restructuring.
Reporter:	Luigi Rizzo (rizzo)	Labels:
Date Opened:	2005-11-25 11:46:25.000-0600	Date Closed:	2006-11-04 09:17:32.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Applications/app_dial
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 1.189.patch ( 1) 1.190.patch ( 2) app_dial.c
Description:	the body of the main function  dial_exec_full() in app_dial.c has grown far too much to be manageable - it is over 850 lines. This is an attempt to clean up the code a little bit, remove the replications, localize variables, etc. - the usual stuff... It would not make sense to supply the diff - it would be unreadable and impossible to review - so i am submitting the whole new file constructed starting from rev 1.188. Detailed list of changes: - removed some large and mostly self-contained blocks of code from  wait_for_answer() and dial_exec_full(), and moved to separate functions; - replaced a largish block of code (now in do_forward_base() that deals  with call forwarding and was repeated twice; - introduced a couple of macros for common string handling tasks  (free previous value if not null, replace with a checked strdup() value); - reduced the scope of variables to the block where they are actually used. ****** ADDITIONAL INFORMATION ****** There are still a number of issues inherited from the original code e.g. errors not checked upon ast_play_and_wait(), inefficient lookup of strings, etc.
Comments:	By: Russell Bryant (russell) 2005-12-01 00:10:56.000-0600 Just so that it is noted in this bug ... If ASTERISK-4869 is merged before these changes, we can replace the use of checked_strdup with ast_strdup provided in that patch. By: Russell Bryant (russell) 2005-12-01 00:16:04.000-0600 I also think it would be worthwhile to take these string handling macros and make them available for use in other modules. There have only been two changes to app_dial.c since 1.188.  I have posted a patch for these changes. By: gkloepfer (gkloepfer) 2005-12-12 12:21:46.000-0600 Please see bug ID 0005979 in case it has an impact on your code restructuring.  Thanks! By: Olle Johansson (oej) 2006-01-09 14:42:35.000-0600 I think it's time to put this into a branch for working further on it. Maybe checking if the duplicate code in app-dial and app_queue should be united in res_dial or something... By: Olle Johansson (oej) 2006-01-30 14:16:49.000-0600 Where are we with this patch? By: Luigi Rizzo (rizzo) 2006-01-30 14:24:38.000-0600 since it is a complete replacement for app_dial(), i have renamed the file to app_dial2.c and imported in my branch in the repository http://svn.digium/com/asterisk/team/rizzo/base/apps/app_dial2.c I am tracking changes to the original app_dial.c By: Serge Vecher (serge-v) 2006-05-01 13:51:08 luigi: seems like the majority of changes you have set out to do have been committed to the trunk already. Are there other changes that need to be looked at or is this bug ready for closure? By: Serge Vecher (serge-v) 2006-05-22 13:30:12 ping again. Thanks! By: Serge Vecher (serge-v) 2006-06-19 14:53:55 moving this to post 1.4 as per Luigi's email. By: jmls (jmls) 2006-10-31 10:59:02.000-0600 as per serge-v' comments: luigi: seems like the majority of changes you have set out to do have been committed to the trunk already. Are there other changes that need to be looked at or is this bug ready for closure? By: Luigi Rizzo (rizzo) 2006-11-04 09:17:31.000-0600 closing this one, as i am committing the various pieces of this patch to trunk (no functional changes at all, just code restructuring and bugfixes). I am also noting, in the commit messages, issues that may be relevant for other branches. Once the code is settled in trunk, i suggest to import in 1.4.x as a whole, so we don't have to go through the debugging of the individual changes.