From 8fd6a8863393ab88250ab37d256487b32e2146ed Mon Sep 17 00:00:00 2001 From: Walter Doekes Date: Wed, 14 May 2014 15:41:42 +0000 Subject: [PATCH] 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 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@413897 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- res/res_musiconhold.c | 75 ++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 44 deletions(-) diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c index 970b7db1a3..6154f334fc 100644 --- a/res/res_musiconhold.c +++ b/res/res_musiconhold.c @@ -163,7 +163,6 @@ static int respawn_time = 20; 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 @@ struct moh_files_state { int sample_queue; int pos; int save_pos; - int save_total; char save_pos_filename[PATH_MAX]; }; @@ -461,28 +459,29 @@ static int moh_files_generator(struct ast_channel *chan, void *data, int len, in 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 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params) } } - /* 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; @@ -523,10 +521,6 @@ static void *moh_files_alloc(struct ast_channel *chan, void *params) 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); return state; @@ -1238,13 +1232,6 @@ static int init_files_class(struct mohclass *class) 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 @@ static void moh_class_destructor(void *obj) ao2_lock(class); while ((member = AST_LIST_REMOVE_HEAD(&class->members, list))) { - free(member); + ast_free(member); } ao2_unlock(class); @@ -1715,9 +1702,9 @@ static void moh_class_destructor(void *obj) if (class->filearray) { int i; for (i = 0; i < class->total_files; i++) { - free(class->filearray[i]); + ast_free(class->filearray[i]); } - free(class->filearray); + ast_free(class->filearray); class->filearray = NULL; }