[svn-commits] wdoekes: trunk r413897 - in /trunk: ./ res/res_musiconhold.c

[SVN commits to the Digium repositories]

Author: wdoekes
Date: Wed May 14 10:41:42 2014
New Revision: 413897

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=413897
Log:
res_musiconhold: Minor cleanup.

Fix a few free()'s that should be ast_free()'s. Reverted an old
workaround that isn't necessary. Reorder a tiny bit of code.
Remove a bit of commented-out code.

Review: https://reviewboard.asterisk.org/r/3536/
........

Merged revisions 413894 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 413895 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 413896 from http://svn.asterisk.org/svn/asterisk/branches/12

Modified:
    trunk/   (props changed)
    trunk/res/res_musiconhold.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-12-merged' - no diff available.

Modified: trunk/res/res_musiconhold.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_musiconhold.c?view=diff&rev=413897&r1=413896&r2=413897
==============================================================================
--- trunk/res/res_musiconhold.c (original)
+++ trunk/res/res_musiconhold.c Wed May 14 10:41:42 2014
@@ -163,7 +163,6 @@
 struct moh_files_state {
 	/*! Holds a reference to the MOH class. */
 	struct mohclass *class;
-	char name[MAX_MUSICCLASS];
 	struct ast_format origwfmt;
 	struct ast_format mohwfmt;
 	int announcement;
@@ -171,7 +170,6 @@
 	int sample_queue;
 	int pos;
 	int save_pos;
-	int save_total;
 	char save_pos_filename[PATH_MAX];
 };
 
@@ -461,28 +459,29 @@
 
 	while (state->sample_queue > 0) {
 		ast_channel_lock(chan);
-		if ((f = moh_files_readframe(chan))) {
-			/* We need to be sure that we unlock
-			 * the channel prior to calling
-			 * ast_write. Otherwise, the recursive locking
-			 * that occurs can cause deadlocks when using
-			 * indirect channels, like local channels
-			 */
-			ast_channel_unlock(chan);
-			state->samples += f->samples;
-			state->sample_queue -= f->samples;
-			if (ast_format_cmp(&f->subclass.format, &state->mohwfmt) == AST_FORMAT_CMP_NOT_EQUAL) {
-				ast_format_copy(&state->mohwfmt, &f->subclass.format);
-			}
-			res = ast_write(chan, f);
-			ast_frfree(f);
-			if (res < 0) {
-				ast_log(LOG_WARNING, "Failed to write frame to '%s': %s\n", ast_channel_name(chan), strerror(errno));
-				return -1;
-			}
-		} else {
-			ast_channel_unlock(chan);
-			return -1;	
+		f = moh_files_readframe(chan);
+
+		/* We need to be sure that we unlock
+		 * the channel prior to calling
+		 * ast_write. Otherwise, the recursive locking
+		 * that occurs can cause deadlocks when using
+		 * indirect channels, like local channels
+		 */
+		ast_channel_unlock(chan);
+		if (!f) {
+			return -1;
+		}
+
+		state->samples += f->samples;
+		state->sample_queue -= f->samples;
+		if (ast_format_cmp(&f->subclass.format, &state->mohwfmt) == AST_FORMAT_CMP_NOT_EQUAL) {
+			ast_format_copy(&state->mohwfmt, &f->subclass.format);
+		}
+		res = ast_write(chan, f);
+		ast_frfree(f);
+		if (res < 0) {
+			ast_log(LOG_WARNING, "Failed to write frame to '%s': %s\n", ast_channel_name(chan), strerror(errno));
+			return -1;
 		}
 	}
 	return res;
@@ -507,12 +506,11 @@
 		}
 	}
 
-	/* LOGIC: Comparing an unrefcounted pointer is a really bad idea, because
-	 * malloc may allocate a different class to the same memory block.  This
-	 * might only happen when two reloads are generated in a short period of
-	 * time, but it's still important to protect against.
-	 * PROG: Compare the quick operation first, to save CPU. */
-	if (state->save_total != class->total_files || strcmp(state->name, class->name) != 0) {
+	/* class is reffed, so we can safely compare it against the (possibly
+	 * recently unreffed) state->class. The unref was done after the ref
+	 * of class, so we're sure that they won't point to the same memory
+	 * by accident. */
+	if (state->class != class) {
 		memset(state, 0, sizeof(*state));
 		if (ast_test_flag(class, MOH_RANDOMIZE) && class->total_files) {
 			state->pos = ast_random() % class->total_files;
@@ -522,10 +520,6 @@
 	state->class = mohclass_ref(class, "Reffing music class for channel");
 	ast_format_copy(&state->origwfmt, ast_channel_writeformat(chan));
 	ast_format_copy(&state->mohwfmt, ast_channel_writeformat(chan));
-
-	/* For comparison on restart of MOH (see above) */
-	ast_copy_string(state->name, class->name, sizeof(state->name));
-	state->save_total = class->total_files;
 
 	moh_post_start(chan, class->name);
 
@@ -1237,13 +1231,6 @@
 			class->dir, class->name);
 		return -1;
 	}
-
-#if 0
-	/* XXX This isn't correct.  Args is an application for custom mode. XXX */
-	if (strchr(class->args, 'r')) {
-		ast_set_flag(class, MOH_RANDOMIZE);
-	}
-#endif
 
 	return 0;
 }
@@ -1654,7 +1641,7 @@
 
 	ao2_lock(class);
 	while ((member = AST_LIST_REMOVE_HEAD(&class->members, list))) {
-		free(member);
+		ast_free(member);
 	}
 	ao2_unlock(class);
 
@@ -1715,9 +1702,9 @@
 	if (class->filearray) {
 		int i;
 		for (i = 0; i < class->total_files; i++) {
-			free(class->filearray[i]);
-		}
-		free(class->filearray);
+			ast_free(class->filearray[i]);
+		}
+		ast_free(class->filearray);
 		class->filearray = NULL;
 	}