<p>Friendly Automation <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14771">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_musiconhold.c: Use ast_file_read_dir to scan MoH directory<br><br>Two changes of note in this patch:<br><br>* Use ast_file_read_dir instead of opendir/readdir/closedir<br><br>* If the files list should be sorted, do that at the end rather than as<br>  we go which improves performance for large lists<br><br>Change-Id: Ic7e9c913c0f85754c99c74c9cf6dd3514b1b941f<br>---<br>M res/res_musiconhold.c<br>1 file changed, 71 insertions(+), 60 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c</span><br><span>index dd8dba5..3749d7b 100644</span><br><span>--- a/res/res_musiconhold.c</span><br><span>+++ b/res/res_musiconhold.c</span><br><span>@@ -1208,15 +1208,71 @@</span><br><span>      }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int on_moh_file(const char *directory, const char *filename, void *obj)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_vector_string *files = obj;</span><br><span style="color: hsl(120, 100%, 40%);">+        char *full_path;</span><br><span style="color: hsl(120, 100%, 40%);">+      char *extension;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    /* Skip files that starts with a dot */</span><br><span style="color: hsl(120, 100%, 40%);">+       if (*filename == '.') {</span><br><span style="color: hsl(120, 100%, 40%);">+               ast_debug(4, "Skipping '%s/%s' because it starts with a dot\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                     directory, filename);</span><br><span style="color: hsl(120, 100%, 40%);">+         return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* We can't do anything with files that don't have an extension,</span><br><span style="color: hsl(120, 100%, 40%);">+       * so check that first and punt if we can't find something */</span><br><span style="color: hsl(120, 100%, 40%);">+     extension = strrchr(filename, '.');</span><br><span style="color: hsl(120, 100%, 40%);">+   if (!extension) {</span><br><span style="color: hsl(120, 100%, 40%);">+             ast_debug(4, "Skipping '%s/%s' because it doesn't have an extension\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                 directory, filename);</span><br><span style="color: hsl(120, 100%, 40%);">+         return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* The extension needs at least two characters (after the .) to be useful */</span><br><span style="color: hsl(120, 100%, 40%);">+  if (strlen(extension) < 3) {</span><br><span style="color: hsl(120, 100%, 40%);">+               ast_debug(4, "Skipping '%s/%s' because it doesn't have at least a two "</span><br><span style="color: hsl(120, 100%, 40%);">+                 "character extension\n", directory, filename);</span><br><span style="color: hsl(120, 100%, 40%);">+              return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* Build the full path (excluding the extension) */</span><br><span style="color: hsl(120, 100%, 40%);">+   if (ast_asprintf(&full_path, "%s/%.*s",</span><br><span style="color: hsl(120, 100%, 40%);">+                 directory,</span><br><span style="color: hsl(120, 100%, 40%);">+                    (int) (extension - filename), filename) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+             /* If we don't have enough memory to build this path, there is no</span><br><span style="color: hsl(120, 100%, 40%);">+          * point in continuing */</span><br><span style="color: hsl(120, 100%, 40%);">+             return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* If the file is present in multiple formats, ensure we only put it</span><br><span style="color: hsl(120, 100%, 40%);">+   * into the list once. Pretty sure this is O(n^2). */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (AST_VECTOR_GET_CMP(files, &full_path[0], !strcmp)) {</span><br><span style="color: hsl(120, 100%, 40%);">+          ast_free(full_path);</span><br><span style="color: hsl(120, 100%, 40%);">+          return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   if (AST_VECTOR_APPEND(files, full_path)) {</span><br><span style="color: hsl(120, 100%, 40%);">+            /* AST_VECTOR_APPEND() can only fail on allocation failure, so</span><br><span style="color: hsl(120, 100%, 40%);">+                 * we stop iterating */</span><br><span style="color: hsl(120, 100%, 40%);">+               ast_free(full_path);</span><br><span style="color: hsl(120, 100%, 40%);">+          return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+static int moh_filename_strcasecmp(const void *a, const void *b)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+   const char **s1 = (const char **) a;</span><br><span style="color: hsl(120, 100%, 40%);">+  const char **s2 = (const char **) b;</span><br><span style="color: hsl(120, 100%, 40%);">+  return strcasecmp(*s1, *s2);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int moh_scan_files(struct mohclass *class) {</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    DIR *files_DIR;</span><br><span style="color: hsl(0, 100%, 40%);">- struct dirent *files_dirent;</span><br><span>         char dir_path[PATH_MAX - sizeof(class->dir)];</span><br><span style="color: hsl(0, 100%, 40%);">-        char filepath[PATH_MAX];</span><br><span style="color: hsl(0, 100%, 40%);">-        char *ext;</span><br><span style="color: hsl(0, 100%, 40%);">-      struct stat statbuf;</span><br><span style="color: hsl(0, 100%, 40%);">-    int res;</span><br><span>     struct ast_vector_string *files;</span><br><span> </span><br><span>         if (class->dir[0] != '/') {</span><br><span>@@ -1224,68 +1280,23 @@</span><br><span>     } else {</span><br><span>             ast_copy_string(dir_path, class->dir, sizeof(dir_path));</span><br><span>  }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>  ast_debug(4, "Scanning '%s' for files for class '%s'\n", dir_path, class->name);</span><br><span style="color: hsl(0, 100%, 40%);">-   files_DIR = opendir(dir_path);</span><br><span style="color: hsl(0, 100%, 40%);">-  if (!files_DIR) {</span><br><span style="color: hsl(0, 100%, 40%);">-               ast_log(LOG_WARNING, "Cannot open dir %s or dir does not exist\n", dir_path);</span><br><span style="color: hsl(0, 100%, 40%);">-         return -1;</span><br><span style="color: hsl(0, 100%, 40%);">-      }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   files = moh_file_vector_alloc(16); /* 16 seems like a reasonable default */</span><br><span style="color: hsl(120, 100%, 40%);">+   /* 16 seems like a reasonable default */</span><br><span style="color: hsl(120, 100%, 40%);">+      files = moh_file_vector_alloc(16);</span><br><span>   if (!files) {</span><br><span style="color: hsl(0, 100%, 40%);">-           closedir(files_DIR);</span><br><span>                 return -1;</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   while ((files_dirent = readdir(files_DIR))) {</span><br><span style="color: hsl(0, 100%, 40%);">-           char *filepath_copy;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-            /* The file name must be at least long enough to have the file type extension */</span><br><span style="color: hsl(0, 100%, 40%);">-                if ((strlen(files_dirent->d_name) < 4))</span><br><span style="color: hsl(0, 100%, 40%);">-                   continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               /* Skip files that starts with a dot */</span><br><span style="color: hsl(0, 100%, 40%);">-         if (files_dirent->d_name[0] == '.')</span><br><span style="color: hsl(0, 100%, 40%);">-                  continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               /* Skip files without extensions... they are not audio */</span><br><span style="color: hsl(0, 100%, 40%);">-               if (!strchr(files_dirent->d_name, '.'))</span><br><span style="color: hsl(0, 100%, 40%);">-                      continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               snprintf(filepath, sizeof(filepath), "%s/%s", dir_path, files_dirent->d_name);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-             if (stat(filepath, &statbuf))</span><br><span style="color: hsl(0, 100%, 40%);">-                       continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               if (!S_ISREG(statbuf.st_mode))</span><br><span style="color: hsl(0, 100%, 40%);">-                  continue;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               if ((ext = strrchr(filepath, '.')))</span><br><span style="color: hsl(0, 100%, 40%);">-                     *ext = '\0';</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-            /* if the file is present in multiple formats, ensure we only put it into the list once */</span><br><span style="color: hsl(0, 100%, 40%);">-              if (AST_VECTOR_GET_CMP(files, &filepath[0], !strcmp)) {</span><br><span style="color: hsl(0, 100%, 40%);">-                     continue;</span><br><span style="color: hsl(0, 100%, 40%);">-               }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               filepath_copy = ast_strdup(filepath);</span><br><span style="color: hsl(0, 100%, 40%);">-           if (!filepath_copy) {</span><br><span style="color: hsl(0, 100%, 40%);">-                   break;</span><br><span style="color: hsl(0, 100%, 40%);">-          }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               if (ast_test_flag(class, MOH_SORTALPHA)) {</span><br><span style="color: hsl(0, 100%, 40%);">-                      res = AST_VECTOR_ADD_SORTED(files, filepath_copy, strcasecmp);</span><br><span style="color: hsl(0, 100%, 40%);">-          } else {</span><br><span style="color: hsl(0, 100%, 40%);">-                        res = AST_VECTOR_APPEND(files, filepath_copy);</span><br><span style="color: hsl(0, 100%, 40%);">-          }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">-               if (res) {</span><br><span style="color: hsl(0, 100%, 40%);">-                      ast_free(filepath_copy);</span><br><span style="color: hsl(0, 100%, 40%);">-                        break;</span><br><span style="color: hsl(0, 100%, 40%);">-          }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (ast_file_read_dir(dir_path, on_moh_file, files)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                ao2_ref(files, -1);</span><br><span style="color: hsl(120, 100%, 40%);">+           return -1;</span><br><span>   }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   closedir(files_DIR);</span><br><span style="color: hsl(120, 100%, 40%);">+  if (ast_test_flag(class, MOH_SORTALPHA)) {</span><br><span style="color: hsl(120, 100%, 40%);">+            AST_VECTOR_SORT(files, moh_filename_strcasecmp);</span><br><span style="color: hsl(120, 100%, 40%);">+      }</span><br><span> </span><br><span>        AST_VECTOR_COMPACT(files);</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14771">change 14771</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/14771"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ic7e9c913c0f85754c99c74c9cf6dd3514b1b941f </div>
<div style="display:none"> Gerrit-Change-Number: 14771 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>