[Asterisk-Dev] LOCAL_USER cleanup update

Luigi Rizzo rizzo at icir.org
Wed Jan 11 08:17:50 MST 2006


On Wed, Jan 11, 2006 at 09:00:11AM -0600, Kevin P. Fleming wrote:
> Luigi Rizzo wrote:
> 
> >  1   fix abuses of the 'struct localuser' structure. These are
> >      in app_dial, app_externalivr, app_queue, see description
> >      below;
> 
> How is this an 'abuse'? app_externalivr needs to keep some information 
> related to each user currently in the app, and this structure is 
> allocated for that very purpose.

the info that app_externalivr actually needs seems to be this:

	struct ivr_localuser {
		struct ast_channel *chan;
		AST_LIST_HEAD(playlist, playlist_entry) playlist;
		AST_LIST_HEAD(finishlist, playlist_entry) finishlist;   
		int abort_current_sound;
		int playing_silence;
		int option_autoclear;
	};

and it only happens to share the 'chan' field with the corresponding
"localusers" entry --  see the attached patch that separates the two,
i believe it is correct.

Actually, i suspect that the lifetime of the above info is
limited to a single execution of app_exec, so probably one does
not even need to calloc() the structure, but could just have it
as an automatic variable in app_exec():

	struct ivr_localuser foo, *u = foo; /* use *u to minimize diffs */

makes sense ?

	cheers
	luigi

Index: app_externalivr.c
===================================================================
--- app_externalivr.c	(revision 7982)
+++ app_externalivr.c	(working copy)
@@ -74,9 +74,8 @@
 	char filename[1];
 };
 
-struct localuser {
+struct ivr_localuser {
 	struct ast_channel *chan;
-	struct localuser *next;
 	AST_LIST_HEAD(playlist, playlist_entry) playlist;
 	AST_LIST_HEAD(finishlist, playlist_entry) finishlist;
 	int abort_current_sound;
@@ -87,7 +86,7 @@
 LOCAL_USER_DECL;
 
 struct gen_state {
-	struct localuser *u;
+	struct ivr_localuser *u;
 	struct ast_filestream *stream;
 	struct playlist_entry *current;
 	int sample_queue;
@@ -110,7 +109,7 @@
 
 static void *gen_alloc(struct ast_channel *chan, void *params)
 {
-	struct localuser *u = params;
+	struct ivr_localuser *u = params;
 	struct gen_state *state;
 
 	state = calloc(1, sizeof(*state));
@@ -144,7 +143,7 @@
 /* caller has the playlist locked */
 static int gen_nextfile(struct gen_state *state)
 {
-	struct localuser *u = state->u;
+	struct ivr_localuser *u = state->u;
 	char *file_to_stream;
 	
 	u->abort_current_sound = 0;
@@ -176,7 +175,7 @@
 static struct ast_frame *gen_readframe(struct gen_state *state)
 {
 	struct ast_frame *f = NULL;
-	struct localuser *u = state->u;
+	struct ivr_localuser *u = state->u;
 	
 	if (u->abort_current_sound ||
 	    (u->playing_silence && AST_LIST_FIRST(&u->playlist))) {
@@ -235,7 +234,7 @@
 {
 	struct playlist_entry *entry;
 
-	entry = calloc(1, sizeof(*entry) + strlen(filename) + 10);
+	entry = calloc(1, sizeof(*entry) + strlen(filename) + 10); /* XXX why 10 ? */
 
 	if (!entry)
 		return NULL;
@@ -247,7 +246,7 @@
 
 static int app_exec(struct ast_channel *chan, void *data)
 {
-	struct localuser *u = NULL;
+	struct localuser *lu = NULL;
 	struct playlist_entry *entry;
 	const char *args = data;
 	int child_stdin[2] = { 0,0 };
@@ -262,22 +261,31 @@
 	FILE *child_commands = NULL;
 	FILE *child_errors = NULL;
 	FILE *child_events = NULL;
+	struct ivr_localuser *u;
 
-	LOCAL_USER_ADD(u);
+	u = calloc(1, sizeof(*u));
+	if (u == NULL) {
+		ast_log(LOG_WARNING, "no memory for ivr_localuser\n");
+		return -1;
+	}
+		
+	LOCAL_USER_ADD(lu);
 	
 	AST_LIST_HEAD_INIT(&u->playlist);
 	AST_LIST_HEAD_INIT(&u->finishlist);
 	u->abort_current_sound = 0;
+	u->chan = chan;
 	
 	if (ast_strlen_zero(args)) {
 		ast_log(LOG_WARNING, "ExternalIVR requires a command to execute\n");
-		goto exit;
+		LOCAL_USER_REMOVE(lu);
+		return -1;
 	}
 
 	buf = ast_strdupa(data);
 	if (!buf) {
 		ast_log(LOG_ERROR, "Out of memory!\n");
-		LOCAL_USER_REMOVE(u);
+		LOCAL_USER_REMOVE(lu);
 		return -1;
 	}
 
@@ -547,8 +555,10 @@
 	while ((entry = AST_LIST_REMOVE_HEAD(&u->playlist, list)))
 		free(entry);
 
-	LOCAL_USER_REMOVE(u);
+	free(u);
 
+	LOCAL_USER_REMOVE(lu);
+
 	return res;
 }
 



More information about the asterisk-dev mailing list