summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKevin Harwell <kharwell@digium.com>2016-10-28 15:11:35 -0500
committerKevin Harwell <kharwell@digium.com>2016-11-04 13:58:21 -0500
commitbd4d7d8ad05bbe8d86b89429dfa55fbc3a0c108d (patch)
tree5da8f30e8611577b74ed2f1237d1e1407b73fc27
parent57a9797e0ac51eae3b8f3254627646b9698a60d2 (diff)
stasis_recording/stored: remove calls to deprecated readdir_r function.
The readdir_r function has been deprecated and should no longer be used. This patch removes the readdir_r dependency (replaced it with readdir) and also moves the directory search code to a more centralized spot (file.c) Also removed a strict dependency on the dirent structure's d_type field as it is not portable. The code now checks to see if the value is available. If so, it tries to use it, but defaults back to using the stats function if necessary. Lastly, for most implementations of readdir it *should* be thread-safe to make concurrent calls to it as long as different directory streams are specified. glibc falls into this category. However, since it is possible that there exist some implementations that are not safe, locking has been added for those other than glibc. ASTERISK-26412 ASTERISK-26509 #close Change-Id: Id8f54689b1e2873e82a09d0d0d2faf41964e80ba
-rw-r--r--include/asterisk/file.h28
-rw-r--r--main/file.c144
-rw-r--r--res/stasis_recording/stored.c217
-rw-r--r--tests/test_file.c183
4 files changed, 421 insertions, 151 deletions
diff --git a/include/asterisk/file.h b/include/asterisk/file.h
index c71866ead..01e5797f5 100644
--- a/include/asterisk/file.h
+++ b/include/asterisk/file.h
@@ -138,6 +138,34 @@ int ast_filedelete(const char *filename, const char *fmt);
int ast_filecopy(const char *oldname, const char *newname, const char *fmt);
/*!
+ * \brief Callback called for each file found when reading directories
+ * \param dir_name the name of the directory
+ * \param filename the name of the file
+ * \param obj user data object
+ * \return non-zero to stop reading, otherwise zero to continue
+ */
+typedef int (*ast_file_on_file)(const char *dir_name, const char *filename, void *obj);
+
+/*!
+ * \brief Recursively iterate through files and directories up to max_depth
+ * \param dir_name the name of the directory to search
+ * \param on_file callback called on each file
+ * \param obj user data object
+ * \param max_depth re-curse into sub-directories up to a given maximum (-1 = infinite)
+ * \return -1 or errno on failure, otherwise 0
+ */
+int ast_file_read_dirs(const char *dir_name, ast_file_on_file on_file, void *obj, int max_depth);
+
+/*!
+ * \brief Iterate over each file in a given directory
+ * \param dir_name the name of the directory to search
+ * \param on_file callback called on each file
+ * \param obj user data object
+ * \return -1 or errno on failure, otherwise 0
+ */
+#define ast_file_read_dir(dir_name, on_file, obj) ast_file_read_dirs(dir_name, on_file, obj, 1)
+
+/*!
* \brief Waits for a stream to stop or digit to be pressed
* \param c channel to waitstream on
* \param breakon string of DTMF digits to break upon
diff --git a/main/file.c b/main/file.c
index 1c51177c2..dc188b0bb 100644
--- a/main/file.c
+++ b/main/file.c
@@ -1095,6 +1095,150 @@ int ast_filecopy(const char *filename, const char *filename2, const char *fmt)
return filehelper(filename, filename2, fmt, ACTION_COPY);
}
+static int __ast_file_read_dirs(struct ast_str **path, ast_file_on_file on_file,
+ void *obj, int max_depth)
+{
+ DIR *dir;
+ struct dirent *entry;
+ size_t size;
+ int res;
+
+ if (!(dir = opendir(ast_str_buffer(*path)))) {
+ ast_log(LOG_ERROR, "Error opening directory - %s: %s\n",
+ ast_str_buffer(*path), strerror(errno));
+ return -1;
+ }
+ size = ast_str_strlen(*path);
+
+ --max_depth;
+
+ res = 0;
+
+ while ((entry = readdir(dir)) != NULL && !errno) {
+ int is_file, is_dir, used_stat = 0;
+
+ if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, "..")) {
+ continue;
+ }
+
+/*
+ * If the dirent structure has a d_type use it to determine if we are dealing with
+ * a file or directory. Unfortunately if it doesn't have it, or if the type is
+ * unknown, or a link then we'll need to use the stat function instead.
+ */
+#ifdef _DIRENT_HAVE_D_TYPE
+ if (entry->d_type != DT_UNKNOWN && entry->d_type != DT_LNK) {
+ is_file = entry->d_type == DT_REG;
+ is_dir = entry->d_type == DT_DIR;
+ } else
+#endif
+ {
+ struct stat statbuf;
+
+ /*
+ * If using the stat function the file needs to be appended to the
+ * path so it can be found. However, before appending make sure the
+ * path contains only the directory for this depth level.
+ */
+ ast_str_truncate(*path, size);
+ ast_str_append(path, 0, "/%s", entry->d_name);
+
+ if (stat(ast_str_buffer(*path), &statbuf)) {
+ ast_log(LOG_ERROR, "Error reading path stats - %s: %s\n",
+ ast_str_buffer(*path), strerror(errno));
+ /*
+ * Output an error, but keep going. It could just be
+ * a broken link and other files could be fine.
+ */
+ continue;
+ }
+
+ is_file = S_ISREG(statbuf.st_mode);
+ is_dir = S_ISDIR(statbuf.st_mode);
+ used_stat = 1;
+ }
+
+ if (is_file) {
+ /* If the handler returns non-zero then stop */
+ if ((res = on_file(ast_str_buffer(*path), entry->d_name, obj))) {
+ break;
+ }
+ /* Otherwise move on to next item in directory */
+ continue;
+ }
+
+ if (!is_dir) {
+ ast_debug(5, "Skipping %s: not a regular file or directory\n",
+ ast_str_buffer(*path));
+ continue;
+ }
+
+ /* Only re-curse into sub-directories if not at the max depth */
+ if (max_depth != 0) {
+ /*
+ * If the stat function was used then the sub-directory has
+ * already been appended, otherwise append it.
+ */
+ if (!used_stat) {
+ ast_str_truncate(*path, size);
+ ast_str_append(path, 0, "/%s", entry->d_name);
+ }
+
+ if ((res = __ast_file_read_dirs(path, on_file, obj, max_depth))) {
+ break;
+ }
+ }
+ }
+
+ closedir(dir);
+
+ if (!res && errno) {
+ ast_log(LOG_ERROR, "Error while reading directories - %s: %s\n",
+ ast_str_buffer(*path), strerror(errno));
+ res = -1;
+ }
+
+ return res;
+}
+
+#if !defined(__GLIBC__)
+/*!
+ * \brief Lock to hold when iterating over directories.
+ *
+ * Currently, 'readdir' is not required to be thread-safe. In most modern implementations
+ * it should be safe to make concurrent calls into 'readdir' that specify different directory
+ * streams (glibc would be one of these). However, since it is potentially unsafe for some
+ * implementations we'll use our own locking in order to achieve synchronization for those.
+ */
+AST_MUTEX_DEFINE_STATIC(read_dirs_lock);
+#endif
+
+int ast_file_read_dirs(const char *dir_name, ast_file_on_file on_file, void *obj, int max_depth)
+{
+ struct ast_str *path;
+ int res;
+
+ if (!(path = ast_str_create(256))) {
+ return -1;
+ }
+
+ ast_str_set(&path, 0, "%s", dir_name);
+ errno = 0;
+
+#if !defined(__GLIBC__)
+ ast_mutex_lock(&read_dirs_lock);
+#endif
+
+ res = __ast_file_read_dirs(&path, on_file, obj, max_depth);
+
+#if !defined(__GLIBC__)
+ ast_mutex_unlock(&read_dirs_lock);
+#endif
+
+ ast_free(path);
+ return res;
+}
+
int ast_streamfile(struct ast_channel *chan, const char *filename, const char *preflang)
{
struct ast_filestream *fs;
diff --git a/res/stasis_recording/stored.c b/res/stasis_recording/stored.c
index 59c07f8de..ab056878d 100644
--- a/res/stasis_recording/stored.c
+++ b/res/stasis_recording/stored.c
@@ -31,7 +31,6 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
#include "asterisk/paths.h"
#include "asterisk/stasis_app_recording.h"
-#include <dirent.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
@@ -122,12 +121,47 @@ static int split_path(const char *path, char **dir, char **file)
return 0;
}
-static void safe_closedir(DIR *dirp)
+struct match_recording_data {
+ const char *file;
+ char *file_with_ext;
+};
+
+static int is_recording(const char *filename)
{
- if (!dirp) {
- return;
+ const char *ext = strrchr(filename, '.');
+
+ if (!ext) {
+ /* No file extension; not us */
+ return 0;
+ }
+ ++ext;
+
+ if (!ast_get_format_for_file_ext(ext)) {
+ ast_debug(5, "Recording %s: unrecognized format %s\n",
+ filename, ext);
+ /* Keep looking */
+ return 0;
}
- closedir(dirp);
+
+ /* Return the index to the .ext */
+ return ext - filename - 1;
+}
+
+static int handle_find_recording(const char *dir_name, const char *filename, void *obj)
+{
+ struct match_recording_data *data = obj;
+ int num;
+
+ /* If not a recording or the names do not match the keep searching */
+ if (!(num = is_recording(filename)) || strncmp(data->file, filename, num)) {
+ return 0;
+ }
+
+ if (ast_asprintf(&data->file_with_ext, "%s/%s", dir_name, filename)) {
+ return -1;
+ }
+
+ return 1;
}
/*!
@@ -143,46 +177,15 @@ static void safe_closedir(DIR *dirp)
*/
static char *find_recording(const char *dir_name, const char *file)
{
- RAII_VAR(DIR *, dir, NULL, safe_closedir);
- struct dirent entry;
- struct dirent *result = NULL;
- char *ext = NULL;
- char *file_with_ext = NULL;
-
- dir = opendir(dir_name);
- if (!dir) {
- return NULL;
- }
-
- while (readdir_r(dir, &entry, &result) == 0 && result != NULL) {
- ext = strrchr(result->d_name, '.');
+ struct match_recording_data data = {
+ .file = file,
+ .file_with_ext = NULL
+ };
- if (!ext) {
- /* No file extension; not us */
- continue;
- }
- *ext++ = '\0';
-
- if (strcmp(file, result->d_name) == 0) {
- if (!ast_get_format_for_file_ext(ext)) {
- ast_log(LOG_WARNING,
- "Recording %s: unrecognized format %s\n",
- result->d_name,
- ext);
- /* Keep looking */
- continue;
- }
- /* We have a winner! */
- break;
- }
- }
+ ast_file_read_dir(dir_name, handle_find_recording, &data);
- if (!result) {
- return NULL;
- }
-
- ast_asprintf(&file_with_ext, "%s/%s.%s", dir_name, file, ext);
- return file_with_ext;
+ /* Note, string potentially allocated in handle_file_recording */
+ return data.file_with_ext;
}
/*!
@@ -238,43 +241,33 @@ static int recording_sort(const void *obj_left, const void *obj_right, int flags
return cmp;
}
-static int scan(struct ao2_container *recordings,
- const char *base_dir, const char *subdir, struct dirent *entry);
-
-static int scan_file(struct ao2_container *recordings,
- const char *base_dir, const char *subdir, const char *filename,
- const char *path)
+static int handle_scan_file(const char *dir_name, const char *filename, void *obj)
{
- RAII_VAR(struct stasis_app_stored_recording *, recording, NULL,
- ao2_cleanup);
- const char *ext;
- char *dot;
-
- ext = strrchr(filename, '.');
+ struct ao2_container *recordings = obj;
+ struct stasis_app_stored_recording *recording;
+ char *dot, *filepath;
- if (!ext) {
- ast_verb(4, " Ignore file without extension: %s\n",
- filename);
- /* No file extension; not us */
+ /* Skip if it is not a recording */
+ if (!is_recording(filename)) {
return 0;
}
- ++ext;
- if (!ast_get_format_for_file_ext(ext)) {
- ast_verb(4, " Not a media file: %s\n", filename);
- /* Not a media file */
- return 0;
+ if (ast_asprintf(&filepath, "%s/%s", dir_name, filename)) {
+ return -1;
}
recording = recording_alloc();
if (!recording) {
+ ast_free(filepath);
return -1;
}
- ast_string_field_set(recording, file_with_ext, path);
-
+ ast_string_field_set(recording, file_with_ext, filepath);
/* Build file and format from full path */
- ast_string_field_set(recording, file, path);
+ ast_string_field_set(recording, file, filepath);
+
+ ast_free(filepath);
+
dot = strrchr(recording->file, '.');
*dot = '\0';
recording->format = dot + 1;
@@ -285,92 +278,14 @@ static int scan_file(struct ao2_container *recordings,
/* Add it to the recordings container */
ao2_link(recordings, recording);
-
- return 0;
-}
-
-static int scan_dir(struct ao2_container *recordings,
- const char *base_dir, const char *subdir, const char *dirname,
- const char *path)
-{
- RAII_VAR(DIR *, dir, NULL, safe_closedir);
- RAII_VAR(struct ast_str *, rel_dirname, NULL, ast_free);
- struct dirent entry;
- struct dirent *result = NULL;
-
- if (strcmp(dirname, ".") == 0 ||
- strcmp(dirname, "..") == 0) {
- ast_verb(4, " Ignoring self/parent dir\n");
- return 0;
- }
-
- /* Build relative dirname */
- rel_dirname = ast_str_create(80);
- if (!rel_dirname) {
- return -1;
- }
- if (!ast_strlen_zero(subdir)) {
- ast_str_append(&rel_dirname, 0, "%s/", subdir);
- }
- if (!ast_strlen_zero(dirname)) {
- ast_str_append(&rel_dirname, 0, "%s", dirname);
- }
-
- /* Read the directory */
- dir = opendir(path);
- if (!dir) {
- ast_log(LOG_WARNING, "Error reading dir '%s'\n", path);
- return -1;
- }
- while (readdir_r(dir, &entry, &result) == 0 && result != NULL) {
- scan(recordings, base_dir, ast_str_buffer(rel_dirname), result);
- }
-
- return 0;
-}
-
-static int scan(struct ao2_container *recordings,
- const char *base_dir, const char *subdir, struct dirent *entry)
-{
- RAII_VAR(struct ast_str *, path, NULL, ast_free);
-
- path = ast_str_create(255);
- if (!path) {
- return -1;
- }
-
- /* Build file path */
- ast_str_append(&path, 0, "%s", base_dir);
- if (!ast_strlen_zero(subdir)) {
- ast_str_append(&path, 0, "/%s", subdir);
- }
- if (entry) {
- ast_str_append(&path, 0, "/%s", entry->d_name);
- }
- ast_verb(4, "Scanning '%s'\n", ast_str_buffer(path));
-
- /* Handle this file */
- switch (entry->d_type) {
- case DT_REG:
- scan_file(recordings, base_dir, subdir, entry->d_name,
- ast_str_buffer(path));
- break;
- case DT_DIR:
- scan_dir(recordings, base_dir, subdir, entry->d_name,
- ast_str_buffer(path));
- break;
- default:
- ast_log(LOG_WARNING, "Skipping %s: not a regular file\n",
- ast_str_buffer(path));
- break;
- }
+ ao2_ref(recording, -1);
return 0;
}
struct ao2_container *stasis_app_stored_recording_find_all(void)
{
- RAII_VAR(struct ao2_container *, recordings, NULL, ao2_cleanup);
+ struct ao2_container *recordings;
int res;
recordings = ao2_container_alloc_rbtree(AO2_ALLOC_OPT_LOCK_NOLOCK,
@@ -379,13 +294,13 @@ struct ao2_container *stasis_app_stored_recording_find_all(void)
return NULL;
}
- res = scan_dir(recordings, ast_config_AST_RECORDING_DIR, "", "",
- ast_config_AST_RECORDING_DIR);
- if (res != 0) {
+ res = ast_file_read_dirs(ast_config_AST_RECORDING_DIR,
+ handle_scan_file, recordings, -1);
+ if (res) {
+ ao2_ref(recordings, -1);
return NULL;
}
- ao2_ref(recordings, +1);
return recordings;
}
diff --git a/tests/test_file.c b/tests/test_file.c
new file mode 100644
index 000000000..4b3d8b6e4
--- /dev/null
+++ b/tests/test_file.c
@@ -0,0 +1,183 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Digium, Inc.
+ *
+ * Kevin Harwell <kharwell@digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*** MODULEINFO
+ <depend>TEST_FRAMEWORK</depend>
+ <support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+
+ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
+
+#include "asterisk/file.h"
+#include "asterisk/paths.h"
+#include "asterisk/test.h"
+#include "asterisk/module.h"
+#include "asterisk/strings.h"
+#include "asterisk/vector.h"
+
+#define FOUND -7
+
+AST_VECTOR(_filenames, struct ast_str *);
+
+static void rm_file(struct ast_str *filename)
+{
+ if (unlink(ast_str_buffer(filename))) {
+ ast_log(LOG_ERROR, "Unable to remove file: %s\n", ast_str_buffer(filename));
+ }
+
+ ast_free(filename);
+}
+
+static int test_files_destroy(struct ast_test *test, char *dir_name,
+ struct _filenames *filenames)
+{
+ int res;
+
+ if (filenames) {
+ AST_VECTOR_CALLBACK_VOID(filenames, rm_file);
+ AST_VECTOR_FREE(filenames);
+ }
+
+ if ((res = rmdir(dir_name)) < 0) {
+ ast_test_status_update(test, "Failed to remove directory: %s\n", dir_name);
+ }
+
+ return res;
+}
+
+static int test_files_create(struct ast_test *test, char *dir_name,
+ struct _filenames *filenames, int num)
+{
+ int i;
+
+ if (!(mkdtemp(dir_name))) {
+ ast_test_status_update(test, "Failed to create directory: %s\n", dir_name);
+ return -1;
+ }
+
+
+ AST_VECTOR_INIT(filenames, num);
+
+ /*
+ * Create "num" files under the specified directory
+ */
+ for (i = 0; i < num; ++i) {
+ int fd;
+ struct ast_str *filename = ast_str_create(32);
+
+ if (!filename) {
+ break;
+ }
+
+ ast_str_set(&filename, 0, "%s/XXXXXX", dir_name);
+
+ fd = mkstemp(ast_str_buffer(filename));
+ if (fd < 0) {
+ ast_test_status_update(test, "Failed to create file: %s\n",
+ ast_str_buffer(filename));
+ ast_free(filename);
+ break;
+ }
+ close(fd);
+
+ AST_VECTOR_APPEND(filenames, filename);
+ }
+
+ if (i != num) {
+ test_files_destroy(test, dir_name, filenames);
+ return -1;
+ }
+
+ return 0;
+}
+
+static char *test_files_get_one(struct _filenames *filenames, int num)
+{
+ /* Every file is in a directory and contains a '/' so okay to do this */
+ return strrchr(ast_str_buffer(
+ AST_VECTOR_GET(filenames, ast_random() % (num - 1))), '/') + 1;
+}
+
+static int handle_find_file(const char *dir_name, const char *filename, void *obj)
+{
+ /* obj contains the name of the file we are looking for */
+ return strcmp(obj, filename) ? 0 : FOUND;
+}
+
+AST_TEST_DEFINE(read_dirs_test)
+{
+ char tmp_dir[] = "/tmp/tmpdir.XXXXXX";
+ struct ast_str *tmp_sub_dir;
+ struct _filenames filenames;
+ enum ast_test_result_state res;
+ const int num_files = 10 + (ast_random() % 10); /* 10-19 random files */
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = "read_dir_test";
+ info->category = "/main/file/";
+ info->summary = "Read a directory's content";
+ info->description = "Iterate over directories looking for a file.";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ /*
+ * We want to test recursively searching into a subdirectory, so
+ * create a top level tmp directory where we will start the search.
+ */
+ if (!(mkdtemp(tmp_dir))) {
+ ast_test_status_update(test, "Failed to create directory: %s\n", tmp_dir);
+ return AST_TEST_FAIL;
+ }
+
+ tmp_sub_dir = ast_str_alloca(32);
+ ast_str_set(&tmp_sub_dir, 0, "%s/XXXXXX", tmp_dir);
+
+ if (test_files_create(test, ast_str_buffer(tmp_sub_dir), &filenames, num_files)) {
+ test_files_destroy(test, tmp_dir, NULL);
+ return AST_TEST_FAIL;
+ }
+
+ res = ast_file_read_dirs(tmp_dir, handle_find_file, test_files_get_one(
+ &filenames, num_files), 2) == FOUND ? AST_TEST_PASS : AST_TEST_FAIL;
+
+ if (test_files_destroy(test, ast_str_buffer(tmp_sub_dir), &filenames) ||
+ test_files_destroy(test, tmp_dir, NULL)) {
+ res = AST_TEST_FAIL;
+ }
+
+ return res;
+}
+
+static int unload_module(void)
+{
+ AST_TEST_UNREGISTER(read_dirs_test);
+ return 0;
+}
+
+static int load_module(void)
+{
+ AST_TEST_REGISTER(read_dirs_test);
+ return AST_MODULE_LOAD_SUCCESS;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "File test module");