Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of jorton@redhat.com designates
 66.187.233.31 as permitted sender)
Date: Mon, 11 Aug 2008 20:20:40 +0100
From: Joe Orton <jorton@redhat.com>
To: dev@httpd.apache.org
Subject: Re: svn commit: r683626 - in /httpd/httpd/trunk: CHANGES
	modules/dav/fs/repos.c
Message-ID: <20080811192040.GA4840@redhat.com>
Mail-Followup-To: dev@httpd.apache.org
References: <20080807151200.A751623889BB@eris.apache.org>
 <20080808092801.GA7196@redhat.com>
 <cc67648e0808080642v1d2769b5u5de0fd549119c820@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <cc67648e0808080642v1d2769b5u5de0fd549119c820@mail.gmail.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
Organization: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor,
 Berkshire, SL4 1TE, United Kingdom.
  Registered in UK and Wales under Company Registration No. 03798903
  Directors: Michael Cunningham (USA), Brendan Lane (Ireland),
 Matt Parson (USA), Charlie Peters (USA)

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 */