[Asterisk-code-review] res_musiconhold: Use a vector instead of custom array allocation (...asterisk[17])
George Joseph
asteriskteam at digium.com
Tue Aug 6 11:05:39 CDT 2019
George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/11655 )
Change subject: res_musiconhold: Use a vector instead of custom array allocation
......................................................................
res_musiconhold: Use a vector instead of custom array allocation
Change-Id: Ic476a56608b1820ca93dcf68d10cd76fc0b94141
---
M res/res_musiconhold.c
1 file changed, 55 insertions(+), 87 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index b598f08..32e77dd 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -171,12 +171,8 @@
char announcement[256];
char mode[80];
char digit;
- /*! A dynamically sized array to hold the list of filenames in "files" mode */
- char **filearray;
- /*! The current size of the filearray */
- int allowed_files;
- /*! The current number of files loaded into the filearray */
- int total_files;
+ /*! A vector of filenames in "files" mode */
+ struct ast_vector_string files;
unsigned int flags;
/*! The format from the MOH source, not applicable to "files" mode */
struct ast_format *format;
@@ -318,6 +314,7 @@
{
struct moh_files_state *state = ast_channel_music_state(chan);
int tries;
+ size_t file_count;
/* Discontinue a stream if it is running already */
if (ast_channel_stream(chan)) {
@@ -335,7 +332,8 @@
state->announcement = 0;
}
- if (!state->class->total_files) {
+ file_count = AST_VECTOR_SIZE(&state->class->files);
+ if (!file_count) {
ast_log(LOG_WARNING, "No files available for class '%s'\n", state->class->name);
return -1;
}
@@ -343,15 +341,15 @@
if (state->pos == 0 && ast_strlen_zero(state->save_pos_filename)) {
/* First time so lets play the file. */
state->save_pos = -1;
- } else if (state->save_pos >= 0 && state->save_pos < state->class->total_files && !strcmp(state->class->filearray[state->save_pos], state->save_pos_filename)) {
+ } else if (state->save_pos >= 0 && state->save_pos < file_count && !strcmp(AST_VECTOR_GET(&state->class->files, state->save_pos), state->save_pos_filename)) {
/* If a specific file has been saved confirm it still exists and that it is still valid */
state->pos = state->save_pos;
state->save_pos = -1;
} else if (ast_test_flag(state->class, MOH_SORTMODE) == MOH_RANDOMIZE) {
/* Get a random file and ensure we can open it */
for (tries = 0; tries < 20; tries++) {
- state->pos = ast_random() % state->class->total_files;
- if (ast_fileexists(state->class->filearray[state->pos], NULL, NULL) > 0) {
+ state->pos = ast_random() % file_count;
+ if (ast_fileexists(AST_VECTOR_GET(&state->class->files, state->pos), NULL, NULL) > 0) {
break;
}
}
@@ -360,29 +358,29 @@
} else {
/* This is easy, just increment our position and make sure we don't exceed the total file count */
state->pos++;
- state->pos %= state->class->total_files;
+ state->pos %= file_count;
state->save_pos = -1;
state->samples = 0;
}
- for (tries = 0; tries < state->class->total_files; ++tries) {
- if (ast_openstream_full(chan, state->class->filearray[state->pos], ast_channel_language(chan), 1)) {
+ for (tries = 0; tries < file_count; ++tries) {
+ if (ast_openstream_full(chan, AST_VECTOR_GET(&state->class->files, state->pos), ast_channel_language(chan), 1)) {
break;
}
- ast_log(LOG_WARNING, "Unable to open file '%s': %s\n", state->class->filearray[state->pos], strerror(errno));
+ ast_log(LOG_WARNING, "Unable to open file '%s': %s\n", AST_VECTOR_GET(&state->class->files, state->pos), strerror(errno));
state->pos++;
- state->pos %= state->class->total_files;
+ state->pos %= file_count;
}
- if (tries == state->class->total_files) {
+ if (tries == file_count) {
return -1;
}
/* Record the pointer to the filename for position resuming later */
- ast_copy_string(state->save_pos_filename, state->class->filearray[state->pos], sizeof(state->save_pos_filename));
+ ast_copy_string(state->save_pos_filename, AST_VECTOR_GET(&state->class->files, state->pos), sizeof(state->save_pos_filename));
- ast_debug(1, "%s Opened file %d '%s'\n", ast_channel_name(chan), state->pos, state->class->filearray[state->pos]);
+ ast_debug(1, "%s Opened file %d '%s'\n", ast_channel_name(chan), state->pos, state->save_pos_filename);
if (state->samples) {
size_t loc;
@@ -490,6 +488,7 @@
{
struct moh_files_state *state;
struct mohclass *class = params;
+ size_t file_count;
state = ast_channel_music_state(chan);
if (!state && (state = ast_calloc(1, sizeof(*state)))) {
@@ -505,14 +504,16 @@
}
}
+ file_count = AST_VECTOR_SIZE(&class->files);
+
/* Resume MOH from where we left off last time or start from scratch? */
- if (state->save_total != class->total_files || strcmp(state->name, class->name) != 0) {
+ if (state->save_total != file_count || strcmp(state->name, class->name) != 0) {
/* Start MOH from scratch. */
ao2_cleanup(state->origwfmt);
ao2_cleanup(state->mohwfmt);
memset(state, 0, sizeof(*state));
- if (ast_test_flag(class, MOH_RANDOMIZE) && class->total_files) {
- state->pos = ast_random() % class->total_files;
+ if (ast_test_flag(class, MOH_RANDOMIZE) && file_count) {
+ state->pos = ast_random() % file_count;
}
}
@@ -522,7 +523,7 @@
ao2_replace(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;
+ state->save_total = file_count;
moh_post_start(chan, class->name);
@@ -1131,45 +1132,6 @@
}
}
-static int moh_add_file(struct mohclass *class, const char *filepath)
-{
- if (!class->allowed_files) {
- class->filearray = ast_calloc(1, INITIAL_NUM_FILES * sizeof(*class->filearray));
- if (!class->filearray) {
- return -1;
- }
- class->allowed_files = INITIAL_NUM_FILES;
- } else if (class->total_files == class->allowed_files) {
- char **new_array;
-
- new_array = ast_realloc(class->filearray, class->allowed_files * sizeof(*class->filearray) * 2);
- if (!new_array) {
- return -1;
- }
- class->filearray = new_array;
- class->allowed_files *= 2;
- }
-
- class->filearray[class->total_files] = ast_strdup(filepath);
- if (!class->filearray[class->total_files]) {
- return -1;
- }
-
- class->total_files++;
-
- return 0;
-}
-
-static int moh_sort_compare(const void *i1, const void *i2)
-{
- char *s1, *s2;
-
- s1 = ((char **)i1)[0];
- s2 = ((char **)i2)[0];
-
- return strcasecmp(s1, s2);
-}
-
static int moh_scan_files(struct mohclass *class) {
DIR *files_DIR;
@@ -1178,7 +1140,7 @@
char filepath[PATH_MAX];
char *ext;
struct stat statbuf;
- int i;
+ int res;
if (class->dir[0] != '/') {
snprintf(dir_path, sizeof(dir_path), "%s/%s", ast_config_AST_DATA_DIR, class->dir);
@@ -1192,12 +1154,11 @@
return -1;
}
- for (i = 0; i < class->total_files; i++) {
- ast_free(class->filearray[i]);
- }
- class->total_files = 0;
+ AST_VECTOR_RESET(&class->files, ast_free);
while ((files_dirent = readdir(files_DIR))) {
+ char *filepath_copy;
+
/* The file name must be at least long enough to have the file type extension */
if ((strlen(files_dirent->d_name) < 4))
continue;
@@ -1222,20 +1183,32 @@
*ext = '\0';
/* if the file is present in multiple formats, ensure we only put it into the list once */
- for (i = 0; i < class->total_files; i++)
- if (!strcmp(filepath, class->filearray[i]))
- break;
+ if (AST_VECTOR_GET_CMP(&class->files, &filepath[0], !strcmp)) {
+ continue;
+ }
- if (i == class->total_files) {
- if (moh_add_file(class, filepath))
- break;
+ filepath_copy = ast_strdup(filepath);
+ if (!filepath_copy) {
+ break;
+ }
+
+ if (ast_test_flag(class, MOH_SORTALPHA)) {
+ res = AST_VECTOR_ADD_SORTED(&class->files, filepath_copy, strcasecmp);
+ } else {
+ res = AST_VECTOR_APPEND(&class->files, filepath_copy);
+ }
+
+ if (res) {
+ ast_free(filepath_copy);
+ break;
}
}
closedir(files_DIR);
- if (ast_test_flag(class, MOH_SORTALPHA))
- qsort(&class->filearray[0], class->total_files, sizeof(char *), moh_sort_compare);
- return class->total_files;
+
+ AST_VECTOR_COMPACT(&class->files);
+
+ return AST_VECTOR_SIZE(&class->files);
}
static int init_files_class(struct mohclass *class)
@@ -1420,6 +1393,7 @@
class->format = ao2_bump(ast_format_slin);
class->srcfd = -1;
class->kill_delay = 100000;
+ AST_VECTOR_INIT(&class->files, 0);
}
return class;
@@ -1614,7 +1588,7 @@
}
if (!state || !state->class || strcmp(mohclass->name, state->class->name)) {
- if (mohclass->total_files) {
+ if (AST_VECTOR_SIZE(&mohclass->files)) {
res = ast_activate_generator(chan, &moh_file_stream, mohclass);
} else {
res = ast_activate_generator(chan, &mohgen, mohclass);
@@ -1693,14 +1667,8 @@
class->srcfd = -1;
}
- if (class->filearray) {
- int i;
- for (i = 0; i < class->total_files; i++) {
- ast_free(class->filearray[i]);
- }
- ast_free(class->filearray);
- class->filearray = NULL;
- }
+ AST_VECTOR_RESET(&class->files, ast_free);
+ AST_VECTOR_FREE(&class->files);
if (class->timer) {
ast_timer_close(class->timer);
@@ -1885,13 +1853,13 @@
for (; (class = ao2_t_iterator_next(&i, "Show files iterator")); mohclass_unref(class, "Unref iterator in moh show files")) {
int x;
- if (!class->total_files) {
+ if (!AST_VECTOR_SIZE(&class->files)) {
continue;
}
ast_cli(a->fd, "Class: %s\n", class->name);
- for (x = 0; x < class->total_files; x++) {
- ast_cli(a->fd, "\tFile: %s\n", class->filearray[x]);
+ for (x = 0; x < AST_VECTOR_SIZE(&class->files); x++) {
+ ast_cli(a->fd, "\tFile: %s\n", AST_VECTOR_GET(&class->files, x));
}
}
ao2_iterator_destroy(&i);
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11655
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: Ic476a56608b1820ca93dcf68d10cd76fc0b94141
Gerrit-Change-Number: 11655
Gerrit-PatchSet: 3
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190806/cd0db217/attachment-0001.html>
More information about the asterisk-code-review
mailing list