[Asterisk-code-review] file.c/ ast file read dirs: Fix issues on filesystems with... (asterisk[master])
George Joseph
asteriskteam at digium.com
Wed Nov 16 14:17:34 CST 2016
George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/4447 )
Change subject: file.c/__ast_file_read_dirs: Fix issues on filesystems without d_type
......................................................................
file.c/__ast_file_read_dirs: Fix issues on filesystems without d_type
One of the code paths in __ast_file_read_dirs will only get executed if
the OS doesn't support dirent->d_type OR if the filesystem the
particular file is on doesn't support it. So, while standard Linux
systems support the field, some filesystems like XFS do not. In this
case, we need to call stat() to determine whether the directory entry
is a file or directory so we append the filename to the supplied
directory path and call stat. We forgot to truncate path back to just
the directory afterwards though so we were passing a complete file name
to the callback in the dir_name parameter instead of just the directory
name.
The logic has been re-written to only create a full_path if we need to
call stat() or if we need to descend into another directory.
Change-Id: I54e4228bd8355fad65200c6df3ec4c9c8a98dfba
---
M main/file.c
M tests/test_file.c
2 files changed, 41 insertions(+), 38 deletions(-)
Approvals:
Kevin Harwell: Looks good to me, but someone else must approve
Mark Michelson: Looks good to me, approved
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, but someone else must approve
diff --git a/main/file.c b/main/file.c
index 37b9e79..fb4ede6 100644
--- a/main/file.c
+++ b/main/file.c
@@ -1093,27 +1093,27 @@
return filehelper(filename, filename2, fmt, ACTION_COPY);
}
-static int __ast_file_read_dirs(struct ast_str **path, ast_file_on_file on_file,
+static int __ast_file_read_dirs(const char *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)))) {
+ if (!(dir = opendir(path))) {
ast_log(LOG_ERROR, "Error opening directory - %s: %s\n",
- ast_str_buffer(*path), strerror(errno));
+ 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;
+ int is_file = 0;
+ int is_dir = 0;
+ RAII_VAR(char *, full_path, NULL, ast_free);
if (!strcmp(entry->d_name, ".") || !strcmp(entry->d_name, "..")) {
continue;
@@ -1128,23 +1128,24 @@
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;
- ast_log(LOG_VERBOSE, "!###### d_name=%s, path=%s, NO USE STAT used_stat=%d\n", entry->d_name, ast_str_buffer(*path), used_stat);
} 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.
+ * Don't use alloca or we risk blowing out the stack if recursing
+ * into subdirectories.
*/
- ast_str_truncate(*path, size);
- ast_str_append(path, 0, "/%s", entry->d_name);
+ full_path = ast_malloc(strlen(path) + strlen(entry->d_name) + 2);
+ if (!full_path) {
+ return -1;
+ }
+ sprintf(full_path, "%s/%s", path, entry->d_name);
- if (stat(ast_str_buffer(*path), &statbuf)) {
+ if (stat(full_path, &statbuf)) {
ast_log(LOG_ERROR, "Error reading path stats - %s: %s\n",
- ast_str_buffer(*path), strerror(errno));
+ full_path, strerror(errno));
/*
* Output an error, but keep going. It could just be
* a broken link and other files could be fine.
@@ -1154,13 +1155,11 @@
is_file = S_ISREG(statbuf.st_mode);
is_dir = S_ISDIR(statbuf.st_mode);
- used_stat = 1;
- ast_log(LOG_VERBOSE, "!###### d_name=%s, path=%s, WE USED IT YO used_stat=%d\n", entry->d_name, ast_str_buffer(*path), used_stat);
}
if (is_file) {
/* If the handler returns non-zero then stop */
- if ((res = on_file(ast_str_buffer(*path), entry->d_name, obj))) {
+ if ((res = on_file(path, entry->d_name, obj))) {
break;
}
/* Otherwise move on to next item in directory */
@@ -1168,25 +1167,22 @@
}
if (!is_dir) {
- ast_debug(5, "Skipping %s: not a regular file or directory\n",
- ast_str_buffer(*path));
+ ast_debug(5, "Skipping %s: not a regular file or directory\n", full_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.
- */
- ast_log(LOG_VERBOSE, "!###### do dir d_name=%s, path=%s, used_stat=%d\n", entry->d_name, ast_str_buffer(*path), used_stat);
- if (!used_stat) {
- ast_str_truncate(*path, size);
- ast_str_append(path, 0, "/%s", entry->d_name);
- ast_log(LOG_VERBOSE, "!###### d_name=%s, path=%s\n", entry->d_name, ast_str_buffer(*path));
+ if (!full_path) {
+ /* Don't use alloca. See note above. */
+ full_path = ast_malloc(strlen(path) + strlen(entry->d_name) + 2);
+ if (!full_path) {
+ return -1;
+ }
+ sprintf(full_path, "%s/%s", path, entry->d_name);
}
- if ((res = __ast_file_read_dirs(path, on_file, obj, max_depth))) {
+ if ((res = __ast_file_read_dirs(full_path, on_file, obj, max_depth))) {
break;
}
}
@@ -1196,7 +1192,7 @@
if (!res && errno) {
ast_log(LOG_ERROR, "Error while reading directories - %s: %s\n",
- ast_str_buffer(*path), strerror(errno));
+ path, strerror(errno));
res = -1;
}
@@ -1217,27 +1213,20 @@
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);
+ res = __ast_file_read_dirs(dir_name, on_file, obj, max_depth);
#if !defined(__GLIBC__)
ast_mutex_unlock(&read_dirs_lock);
#endif
- ast_free(path);
return res;
}
diff --git a/tests/test_file.c b/tests/test_file.c
index 64bad92..bb8a995 100644
--- a/tests/test_file.c
+++ b/tests/test_file.c
@@ -21,7 +21,10 @@
<support_level>core</support_level>
***/
+
#include "asterisk.h"
+#include <sys/stat.h>
+#include <stdio.h>
#include "asterisk/file.h"
#include "asterisk/paths.h"
@@ -115,6 +118,17 @@
static int handle_find_file(const char *dir_name, const char *filename, void *obj)
{
+ struct stat statbuf;
+ char *full_path = ast_alloca(strlen(dir_name) + strlen(filename) + 2);
+
+ sprintf(full_path, "%s/%s", dir_name, filename);
+
+ errno = 0;
+ if (stat(full_path, &statbuf)) {
+ ast_log(LOG_ERROR, "Error reading path stats - %s: %s\n",
+ full_path, strerror(errno));
+ return 0;
+ }
/* obj contains the name of the file we are looking for */
return strcmp(obj, filename) ? 0 : FOUND;
}
--
To view, visit https://gerrit.asterisk.org/4447
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I54e4228bd8355fad65200c6df3ec4c9c8a98dfba
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
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-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list