You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by rb...@covalent.net on 2001/01/22 03:56:52 UTC
New directory API...
Will Rowe and I were talking earlier today, and our directory API is
horribly broken and inefficient. Basically, on Unix if you want to get
any information about a file in a directory, we have to do a stat for each
piece of information.
For example, if you want the size, that's one stat. If you then want the
file type, that's another stat. Yuck!
Here is a design that Will suggested earlier today, implemented for
Unix. Basically, it just uses the finfo structure to store information
about the file when you read it from the directory.
? build.log
? build.err
Index: file_io/unix/dir.c
===================================================================
RCS file: /home/cvs/apr/file_io/unix/dir.c,v
retrieving revision 1.42
diff -u -d -b -w -u -r1.42 dir.c
--- file_io/unix/dir.c 2001/01/05 19:40:05 1.42
+++ file_io/unix/dir.c 2001/01/22 02:46:57
@@ -53,6 +53,7 @@
*/
#include "fileio.h"
+#include "apr_file_info.h"
#include "apr_strings.h"
#include "apr_portable.h"
@@ -106,20 +107,18 @@
return rv;
}
-apr_status_t apr_readdir(apr_dir_t *thedir)
+apr_status_t apr_readdir(apr_finfo_t *finfo, apr_dir_t *thedir)
{
#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) \
&& !defined(READDIR_IS_THREAD_SAFE)
apr_status_t ret;
-#endif
-
-#if APR_HAS_THREADS && defined(_POSIX_THREAD_SAFE_FUNCTIONS) \
- && !defined(READDIR_IS_THREAD_SAFE)
ret = readdir_r(thedir->dirstruct, thedir->entry, &thedir->entry);
/* Avoid the Linux problem where at end-of-directory thedir->entry
* is set to NULL, but ret = APR_SUCCESS.
*/
+ finfo->cntxt = thedir->cntxt;
+ finfo->fname = apr_pstrcat(thedir->cntxt, thedir->dirname, "/", thedir->entry->d_name, NULL);
return (ret == APR_SUCCESS && thedir->entry == NULL) ? APR_ENOENT : ret;
#else
@@ -131,6 +130,8 @@
}
return errno;
}
+ finfo->cntxt = thedir->cntxt;
+ finfo->fname = apr_pstrcat(thedir->cntxt, thedir->dirname, "/", thedir->entry->d_name, NULL);
return APR_SUCCESS;
#endif
}
@@ -161,94 +162,6 @@
else {
return errno;
}
-}
-
-apr_status_t apr_dir_entry_size(apr_size_t *size, apr_dir_t *thedir)
-{
- struct stat filestat;
- char *fname = NULL;
-
- if (thedir->entry == NULL) {
- *size = -1;
- return APR_ENOFILE;
- }
- fname = apr_pstrcat(thedir->cntxt, thedir->dirname, "/",
- thedir->entry->d_name, NULL);
- if (stat(fname, &filestat) == -1) {
- *size = 0;
- return errno;
- }
-
- *size = filestat.st_size;
- return APR_SUCCESS;
-}
-
-apr_status_t apr_dir_entry_mtime(apr_time_t *mtime, apr_dir_t *thedir)
-{
- struct stat filestat;
- char *fname = NULL;
-
- if (thedir->entry == NULL) {
- *mtime = -1;
- return APR_ENOFILE;
- }
-
- fname = apr_pstrcat(thedir->cntxt, thedir->dirname, "/",
- thedir->entry->d_name, NULL);
- if (stat(fname, &filestat) == -1) {
- *mtime = -1;
- return errno;
- }
-
- apr_ansi_time_to_apr_time(mtime, filestat.st_mtime);
- return APR_SUCCESS;
-}
-
-apr_status_t apr_dir_entry_ftype(apr_filetype_e *type, apr_dir_t *thedir)
-{
- struct stat filestat;
- char *fname = NULL;
-
- if (thedir->entry == NULL) {
- *type = APR_REG;
- return APR_ENOFILE;
- }
-
- fname = apr_pstrcat(thedir->cntxt, thedir->dirname, "/",
- thedir->entry->d_name, NULL);
- if (stat(fname, &filestat) == -1) {
- *type = APR_REG;
- return errno;
- }
-
- if (S_ISREG(filestat.st_mode))
- *type = APR_REG;
- if (S_ISDIR(filestat.st_mode))
- *type = APR_DIR;
- if (S_ISCHR(filestat.st_mode))
- *type = APR_CHR;
- if (S_ISBLK(filestat.st_mode))
- *type = APR_BLK;
- if (S_ISFIFO(filestat.st_mode))
- *type = APR_PIPE;
- if (S_ISLNK(filestat.st_mode))
- *type = APR_LNK;
-#ifndef BEOS
- if (S_ISSOCK(filestat.st_mode))
- *type = APR_SOCK;
-#endif
- return APR_SUCCESS;
-}
-
-apr_status_t apr_get_dir_filename(const char **new, apr_dir_t *thedir)
-{
- /* Detect End-Of-File */
- if (thedir == NULL || thedir->entry == NULL) {
- *new = NULL;
- return APR_ENOENT;
- }
- (*new) = thedir->entry->d_name;
- return APR_SUCCESS;
}
apr_status_t apr_get_os_dir(apr_os_dir_t **thedir, apr_dir_t *dir)
Index: include/apr_file_info.h
===================================================================
RCS file: /home/cvs/apr/include/apr_file_info.h,v
retrieving revision 1.4
diff -u -d -b -w -u -r1.4 apr_file_info.h
--- include/apr_file_info.h 2001/01/20 22:18:55 1.4
+++ include/apr_file_info.h 2001/01/22 02:46:57
@@ -255,9 +255,9 @@
* Read the next entry from the specified directory.
* @param thedir the directory descriptor to read from, and fill out.
* @tip All systems return . and .. as the first two files.
- * @deffunc apr_status_t apr_readdir(apr_dir_t *thedir)
+ * @deffunc apr_status_t apr_readdir(apr_finfo_t *finfo, apr_dir_t *thedir)
*/
-APR_DECLARE(apr_status_t) apr_readdir(apr_dir_t *thedir);
+APR_DECLARE(apr_status_t) apr_readdir(apr_finfo_t *finfo, apr_dir_t *thedir);
/**
* Rewind the directory to the first entry.
@@ -265,42 +265,6 @@
* @deffunc apr_status_t apr_rewinddir(apr_dir_t *thedir)
*/
APR_DECLARE(apr_status_t) apr_rewinddir(apr_dir_t *thedir);
-
-/**
- * Get the file name of the current directory entry.
- * @param new_path the file name of the directory entry.
- * @param thedir the currently open directory.
- * @deffunc apr_status_t apr_get_dir_filename(const char **new_path, apr_dir_t *thedir)
- */
-APR_DECLARE(apr_status_t) apr_get_dir_filename(const char **new_path,
- apr_dir_t *thedir);
-
-/**
- * Get the size of the current directory entry.
- * @param size the size of the directory entry.
- * @param thedir the currently open directory.
- * @deffunc apr_status_t apr_dir_entry_size(apr_size_t *size, apr_dir_t *thedir)
- */
-APR_DECLARE(apr_status_t) apr_dir_entry_size(apr_size_t *size,
- apr_dir_t *thedir);
-
-/**
- * Get the last modified time of the current directory entry.
- * @param mtime the last modified time of the directory entry.
- * @param thedir the currently open directory.
- * @deffunc apr_status_t apr_dir_entry_mtime(apr_time_t *mtime, apr_dir_t *thedir)
- */
-APR_DECLARE(apr_status_t) apr_dir_entry_mtime(apr_time_t *mtime,
- apr_dir_t *thedir);
-
-/**
- * Get the file type of the current directory entry.
- * @param type the file type of the directory entry.
- * @param thedir the currently open directory.
- * @deffunc apr_status_t apr_dir_entry_ftype(apr_filetype_e *type, apr_dir_t *thedir)
- */
-APR_DECLARE(apr_status_t) apr_dir_entry_ftype(apr_filetype_e *type,
- apr_dir_t *thedir);
#ifdef __cplusplus
}
Index: test/testfile.c
===================================================================
RCS file: /home/cvs/apr/test/testfile.c,v
retrieving revision 1.24
diff -u -d -b -w -u -r1.24 testfile.c
--- test/testfile.c 2001/01/15 19:19:24 1.24
+++ test/testfile.c 2001/01/22 02:46:59
@@ -290,8 +290,7 @@
apr_dir_t *temp;
apr_file_t *file = NULL;
apr_size_t bytes;
- apr_filetype_e type;
- const char *fname;
+ apr_finfo_t finfo;
fprintf(stdout, "Testing Directory functions.\n");
@@ -322,7 +321,7 @@
}
fprintf(stdout, "\tReading Directory.......");
- if ((apr_readdir(temp)) != APR_SUCCESS) {
+ if ((apr_readdir(&finfo, temp)) != APR_SUCCESS) {
fprintf(stderr, "Could not read directory\n");
return -1;
}
@@ -336,29 +335,28 @@
/* Because I want the file I created, I am skipping the "." and ".."
* files that are here.
*/
- if (apr_readdir(temp) != APR_SUCCESS) {
+ if (apr_readdir(&finfo, temp) != APR_SUCCESS) {
fprintf(stderr, "Error reading directory testdir");
return -1;
}
- apr_get_dir_filename(&fname, temp);
- } while (fname[0] == '.');
- if (strcmp(fname, "testfile")) {
- fprintf(stderr, "Got wrong file name %s\n", fname);
+ } while (!strncmp(finfo.fname, "testdir/.", strlen("testdir/.")));
+ if (strcmp(finfo.fname, "testdir/testfile")) {
+ fprintf(stderr, "Got wrong file name %s\n", finfo.fname);
return -1;
}
fprintf(stdout, "OK\n");
+ apr_stat(&finfo, finfo.fname, APR_FINFO_NORM, finfo.cntxt);
+
fprintf(stdout, "\t\tFile type.......");
- apr_dir_entry_ftype(&type, temp);
- if (type != APR_REG) {
+ if (finfo.filetype != APR_REG) {
fprintf(stderr, "Got wrong file type\n");
return -1;
}
fprintf(stdout, "OK\n");
fprintf(stdout, "\t\tFile size.......");
- apr_dir_entry_size(&bytes, temp);
- if (bytes != strlen("Another test!!!")) {
+ if (finfo.size != strlen("Another test!!!")) {
fprintf(stderr, "Got wrong file size %" APR_SIZE_T_FMT "\n", bytes);
return -1;
}
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: New directory API...
Posted by rb...@covalent.net.
> > That is on Unix, where that is all the information we have. Windows,
> > OS/2, and Mac OS X all get more information from their readdir
> > functions. Just because we don't get as much information from Unix
> > doesn't mean we shouldn't use what we do get. When the apr_stat function
> > can do incremental stat's, this API will improve performance on all
> > platforms.
>
> Coolness.
>
> That works for me. However, make sure that the readdir() function sets the
> .valid bits (wasn't in your initial patch).
Whoops. I made a mental note and then forgot about it. Will be in before
we commit.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: New directory API...
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Jan 21, 2001 at 09:38:10PM -0800, rbb@covalent.net wrote:
>
> > > Here is a design that Will suggested earlier today, implemented for
> > > Unix. Basically, it just uses the finfo structure to store information
> > > about the file when you read it from the directory.
> >
> > This isn't using finfo at all. It only returns the file name. The caller is
> > then supposed to go back and use apr_stat for the other data.
> >
> > If we're just going to return the name, then let's do that. Returning an
> > finfo is a misnomer.
>
> That is on Unix, where that is all the information we have. Windows,
> OS/2, and Mac OS X all get more information from their readdir
> functions. Just because we don't get as much information from Unix
> doesn't mean we shouldn't use what we do get. When the apr_stat function
> can do incremental stat's, this API will improve performance on all
> platforms.
Coolness.
That works for me. However, make sure that the readdir() function sets the
.valid bits (wasn't in your initial patch).
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: New directory API...
Posted by rb...@covalent.net.
> > Here is a design that Will suggested earlier today, implemented for
> > Unix. Basically, it just uses the finfo structure to store information
> > about the file when you read it from the directory.
>
> This isn't using finfo at all. It only returns the file name. The caller is
> then supposed to go back and use apr_stat for the other data.
>
> If we're just going to return the name, then let's do that. Returning an
> finfo is a misnomer.
That is on Unix, where that is all the information we have. Windows,
OS/2, and Mac OS X all get more information from their readdir
functions. Just because we don't get as much information from Unix
doesn't mean we shouldn't use what we do get. When the apr_stat function
can do incremental stat's, this API will improve performance on all
platforms.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: New directory API...
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Jan 21, 2001 at 06:56:52PM -0800, rbb@covalent.net wrote:
>
> Will Rowe and I were talking earlier today, and our directory API is
> horribly broken and inefficient. Basically, on Unix if you want to get
> any information about a file in a directory, we have to do a stat for each
> piece of information.
>
> For example, if you want the size, that's one stat. If you then want the
> file type, that's another stat. Yuck!
Ugly... yup.
> Here is a design that Will suggested earlier today, implemented for
> Unix. Basically, it just uses the finfo structure to store information
> about the file when you read it from the directory.
This isn't using finfo at all. It only returns the file name. The caller is
then supposed to go back and use apr_stat for the other data.
If we're just going to return the name, then let's do that. Returning an
finfo is a misnomer.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/