httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: svn commit: r683626 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
Date Mon, 11 Aug 2008 19:20:40 GMT
On Fri, Aug 08, 2008 at 09:42:01AM -0400, Jeff Trawick wrote:
> On Fri, Aug 8, 2008 at 5:28 AM, Joe Orton <jorton@redhat.com> wrote:
> > On Thu, Aug 07, 2008 at 03:12:00PM -0000, Jeff Trawick wrote:
> > > --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
> > > +++ httpd/httpd/trunk/modules/dav/fs/repos.c Thu Aug  7 08:12:00 2008
> > > @@ -1475,10 +1475,8 @@
> > >          /* append this file onto the path buffer (copy null term) */
> > >          dav_buffer_place_mem(pool, &fsctx->path1, dirent.name, len
+ 1,
> > 0);
> > >
> > > -
> > > -        /* ### Optimize me, dirent can give us what we need! */
> > >          status = apr_stat(&fsctx->info1.finfo, fsctx->path1.buf,
> > > -                          APR_FINFO_NORM | APR_FINFO_LINK, pool);
> > > +                          APR_FINFO_TYPE | APR_FINFO_LINK, pool);
> > >          if (status != APR_SUCCESS && status != APR_INCOMPLETE) {
> > >              /* woah! where'd it go? */
> > >              /* ### should have a better error here */
> > >
> >
> > Nit pick: on Unix APR_FINFO_PROT is needed too, to support the
> > executable property....
> 
> plz show me where; I'm blind; thanks!

If I'm understanding things correctly, this stat() call fetches file 
attributes which will be used when returning property information via 
dav_fs_insert_prop.

So I think that the stat() call should mask in all the attribute types 
which are used in dav_fs_insert_prop: ctime, mtime, size, and prot iff 
!WIN32.

Here is a good demo - change APR file_io/unix/filestat.c as below:

static void fill_out_finfo(apr_finfo_t *finfo, struct_stat *info,
                           apr_int32_t wanted)
{ 
#if 0
    finfo->valid = APR_FINFO_MIN | APR_FINFO_IDENT | APR_FINFO_NLINK
                 | APR_FINFO_OWNER | APR_FINFO_PROT;
#else
    finfo->valid = wanted;
#endif

i.e. lose all the file attributes which come for free with stat() but 
weren't explicitly requested; you'll see that mod_dav no longer returns 
the executable property.

(with cadaver, for an existing foo.txt on the server, "chexec + foo.txt" 
then see whether the "*" shows up next to foo.txt in a subsequent "ls")

I think that something like this is the way to go: (against 2.2.x since 
my trunk install is currently refusing to do anything DAVy)

--- repos.c	(revision 683918)
+++ repos.c	(working copy)
@@ -119,9 +119,19 @@
 ** Does this platform support an executable flag?
 **
 ** ### need a way to portably abstract this query
+**
+** DAV_FINFO_MASK gives the appropriate mask to use for the stat call
+** used to get file attributes.
 */
 #ifndef WIN32
 #define DAV_FS_HAS_EXECUTABLE
+#define DAV_FINFO_MASK (APR_FINFO_LINK | APR_FINFO_TYPE | APR_FINFO_INODE | \
+                        APR_FINFO_SIZE | APR_FINFO_CTIME | APR_FINFO_MTIME)
+#else
+/* as above plus APR_FINFO_PROT */
+#define DAV_FINFO_MASK (APR_FINFO_LINK | APR_FINFO_TYPE | APR_FINFO_INODE | \
+                        APR_FINFO_SIZE | APR_FINFO_CTIME | APR_FINFO_MTIME \
+                        APR_FINFO_PROT)
 #endif
 
 /*
@@ -1482,7 +1492,7 @@
 
         /* ### Optimize me, dirent can give us what we need! */
         status = apr_stat(&fsctx->info1.finfo, fsctx->path1.buf,
-                          APR_FINFO_NORM | APR_FINFO_LINK, pool);
+                          DAV_FINFO_MASK, pool);
         if (status != APR_SUCCESS && status != APR_INCOMPLETE) {
             /* woah! where'd it go? */
             /* ### should have a better error here */

Mime
View raw message