Merge "stasis_recording/stored: remove calls to deprecated readdir_r function." into 13

include/asterisk/file.h

@@ -137,6 +137,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

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;

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;
 	}
-	closedir(dirp);
+	++ext;
+
+	if (!ast_get_format_for_file_ext(ext)) {
+		ast_debug(5, "Recording %s: unrecognized format %s\n",
+			filename, ext);
+		/* Keep looking */
+		return 0;
+	}
+
+	/* 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;
+	struct match_recording_data data = {
+		.file = file,
+		.file_with_ext = NULL
+	};
 
-	dir = opendir(dir_name);
-	if (!dir) {
-		return NULL;
-	}
+	ast_file_read_dir(dir_name, handle_find_recording, &data);
 
-	while (readdir_r(dir, &entry, &result) == 0 && result != NULL) {
-		ext = strrchr(result->d_name, '.');
-
-		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;
-		}
-	}
-
-	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;
+	struct ao2_container *recordings = obj;
+	struct stasis_app_stored_recording *recording;
+	char *dot, *filepath;
 
-	ext = strrchr(filename, '.');
-
-	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;
 }
 

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");