From 80d40805c3073f97251595d63cea2a1fff1cf2f1 Mon Sep 17 00:00:00 2001 Date: Tue, 30 Aug 2011 18:10:12 -0700 Subject: [PATCH 4/5] Fix invalid assumption about dirent structure definition. The code assumed that dirent.d_name is statically defined as an array large enough to hold the name. On Solaris this isn't the case: d_name is defined as 'char d_name[1]'. Posix also does not specify that the static definition must be a full-sized array. One implication of this is that dirent's need not all be the same size. The fix here therefore removes both those assumptions. It allocates a buffer that contains a set of pointers to dirent's, followed by the actual dirent's. Furthermore it copies the individual dirent's based on their actual dynamic size, not the static size of the struct. As an optimization it makes use of popular, but non-posix-defined struct members where available to efficiently determine the size, but falls back to the always correct strlen() if these are not available. A side effect of this is that this reduces the memory use quite significantly in most cases. --- source3/configure.in | 2 + source3/m4/aclocal.m4 | 26 ++++++++++++++++++++++++ source3/modules/vfs_dirsort.c | 44 +++++++++++++++++++++++++++++++--------- source3/wscript | 6 +++++ 4 files changed, 68 insertions(+), 10 deletions(-) diff --git a/source3/configure.in b/source3/configure.in index 73c80c3..9ca89bd 100644 --- a/source3/configure.in +++ b/source3/configure.in @@ -822,6 +822,8 @@ AC_STRUCT_ST_RDEV AC_DIRENT_D_OFF AC_DIR_D_FD AC_DIR_DD_FD +AC_DIRENT_D_RECLEN +AC_DIRENT_D_NAMLEN AC_CHECK_TYPE(ssize_t, int) AC_CHECK_TYPE(wchar_t, unsigned short) diff --git a/source3/m4/aclocal.m4 b/source3/m4/aclocal.m4 index 9fb9431..466d19d 100644 --- a/source3/m4/aclocal.m4 +++ b/source3/m4/aclocal.m4 @@ -37,6 +37,32 @@ if test $ac_cv_dir_dd_fd = yes; then fi ]) +dnl test whether dirent has a d_reclen member +AC_DEFUN(AC_DIRENT_D_RECLEN, +[AC_CACHE_CHECK([for d_reclen in dirent], ac_cv_dirent_d_reclen, +[AC_TRY_COMPILE([ +#include +#include +#include ], [struct dirent d; d.d_reclen;], +ac_cv_dirent_d_reclen=yes, ac_cv_dirent_d_reclen=no)]) +if test $ac_cv_dirent_d_reclen = yes; then + AC_DEFINE(HAVE_DIRENT_D_RECLEN,1,[Whether dirent has a d_reclen member]) +fi +]) + +dnl test whether dirent has a d_namlen member +AC_DEFUN(AC_DIRENT_D_NAMLEN, +[AC_CACHE_CHECK([for d_namlen in dirent], ac_cv_dirent_d_namlen, +[AC_TRY_COMPILE([ +#include +#include +#include ], [struct dirent d; d.d_namlen;], +ac_cv_dirent_d_namlen=yes, ac_cv_dirent_d_namlen=no)]) +if test $ac_cv_dirent_d_namlen = yes; then + AC_DEFINE(HAVE_DIRENT_D_NAMLEN,1,[Whether dirent has a d_namlen member]) +fi +]) + dnl Mark specified module as shared dnl SMB_MODULE(name,static_files,shared_files,subsystem,whatif-static,whatif-shared) AC_DEFUN(SMB_MODULE, diff --git a/source3/modules/vfs_dirsort.c b/source3/modules/vfs_dirsort.c index c3070de..38154ff 100644 --- a/source3/modules/vfs_dirsort.c +++ b/source3/modules/vfs_dirsort.c @@ -32,14 +32,27 @@ #error "need either dirfd, DIR.d_fd, or DIR.dd_fd" #endif -static int compare_dirent (const SMB_STRUCT_DIRENT *da, const SMB_STRUCT_DIRENT *db) +#ifdef HAVE_DIRENT_D_RECLEN +#define DIRENT_RECLEN(rec) (rec->d_reclen) +#elif HAVE_DIRENT_D_NAMLEN +#define DIRENT_RECLEN(rec) \ + ((offsetof(SMB_STRUCT_DIRENT, d_name[0]) + \ + (rec->d_namlen) + 1 + 7) & ~7) +#else +#define DIRENT_RECLEN(rec) \ + ((offsetof(SMB_STRUCT_DIRENT, d_name[0]) + \ + (strlen(rec->d_name)) + 1 + 7) & ~7) +#endif + +static int compare_dirent(SMB_STRUCT_DIRENT * const *da, + SMB_STRUCT_DIRENT * const *db) { - return StrCaseCmp(da->d_name, db->d_name); + return StrCaseCmp((*da)->d_name, (*db)->d_name); } struct dirsort_privates { long pos; - SMB_STRUCT_DIRENT *directory_list; + SMB_STRUCT_DIRENT **directory_list; long number_of_entries; time_t mtime; SMB_STRUCT_DIR *source_directory; @@ -60,20 +73,25 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) SMB_STRUCT_DIRENT *dp; struct stat dir_stat; long current_pos; + unsigned dirents_size; + char *next_dirent; + unsigned reclen; struct dirsort_privates *data = NULL; SMB_VFS_HANDLE_GET_DATA(handle, data, struct dirsort_privates, return false); data->number_of_entries = 0; + dirents_size = 0; if (fstat(data->fd, &dir_stat) == 0) { data->mtime = dir_stat.st_mtime; } - while (SMB_VFS_NEXT_READDIR(handle, data->source_directory, NULL) - != NULL) { + while ((dp = SMB_VFS_NEXT_READDIR(handle, data->source_directory, + NULL)) != NULL) { data->number_of_entries++; + dirents_size += sizeof(SMB_STRUCT_DIRENT *) + DIRENT_RECLEN(dp); } /* Open the underlying directory and count the number of entries @@ -82,18 +100,24 @@ static bool open_and_sort_dir (vfs_handle_struct *handle) /* Set up an array and read the directory entries into it */ SAFE_FREE(data->directory_list); /* destroy previous cache if needed */ - data->directory_list = (SMB_STRUCT_DIRENT *)SMB_MALLOC( - data->number_of_entries * sizeof(SMB_STRUCT_DIRENT)); + data->directory_list = (SMB_STRUCT_DIRENT **)SMB_MALLOC(dirents_size); if (!data->directory_list) { return false; } current_pos = data->pos; data->pos = 0; + next_dirent = (char *) &data->directory_list[data->number_of_entries]; while ((dp = SMB_VFS_NEXT_READDIR(handle, data->source_directory, NULL)) != NULL) { - if (data->pos >= data->number_of_entries) + reclen = DIRENT_RECLEN(dp); + if (data->pos >= data->number_of_entries || + next_dirent + reclen > + ((char *) data->directory_list) + dirents_size) break; /* directory changed since opendir */ - data->directory_list[data->pos++] = *dp; + data->directory_list[data->pos++] = + (SMB_STRUCT_DIRENT *) next_dirent; + memcpy(next_dirent, dp, reclen); + next_dirent += reclen; } /* Sort the directory entries by name */ @@ -209,7 +233,7 @@ static SMB_STRUCT_DIRENT *dirsort_readdir(vfs_handle_struct *handle, return NULL; } - return &data->directory_list[data->pos++]; + return data->directory_list[data->pos++]; } static void dirsort_seekdir(vfs_handle_struct *handle, SMB_STRUCT_DIR *dirp, diff --git a/source3/wscript b/source3/wscript index ecbc58a..264889c 100644 --- a/source3/wscript +++ b/source3/wscript @@ -529,6 +529,12 @@ msg.msg_acctrightslen = sizeof(fd); conf.CHECK_STRUCTURE_MEMBER('struct dirent', 'd_off', headers='unistd.h sys/types.h dirent.h', define='HAVE_DIRENT_D_OFF') + conf.CHECK_STRUCTURE_MEMBER('struct dirent', 'd_reclen', + headers='unistd.h sys/types.h dirent.h', + define='HAVE_DIRENT_D_RECLEN') + conf.CHECK_STRUCTURE_MEMBER('struct dirent', 'd_namlen', + headers='unistd.h sys/types.h dirent.h', + define='HAVE_DIRENT_D_NAMLEN') conf.CHECK_FUNCS('setnetgrent getnetgrent endnetgrent') if conf.CHECK_CFLAGS('-Werror-implicit-function-declaration'): -- 1.7.3.4