res_musiconhold: Minor cleanup.
authorWalter Doekes <walter+asterisk@wjd.nu>
Wed, 14 May 2014 15:41:42 +0000 (15:41 +0000)
committerWalter Doekes <walter+asterisk@wjd.nu>
Wed, 14 May 2014 15:41:42 +0000 (15:41 +0000)
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

index 970b7db..6154f33 100644 (file)
@@ -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;
        }