You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2009/10/09 23:41:32 UTC
svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Author: minfrin
Date: Fri Oct 9 21:41:31 2009
New Revision: 823703
URL: http://svn.apache.org/viewvc?rev=823703&view=rev
Log:
mod_dav: Provide a mechanism to obtain the request_rec and pathname
from the dav_resource.
Submitted by: Jari Urpalainen <jari.urpalainen nokia.com>,
Brian France <brian brianfrance.com>
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/modules/dav/fs/repos.c
httpd/httpd/trunk/modules/dav/main/mod_dav.h
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=823703&r1=823702&r2=823703&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Oct 9 21:41:31 2009
@@ -10,6 +10,10 @@
mod_proxy_ftp: NULL pointer dereference on error paths.
[Stefan Fritsch <sf fritsch.de>, Joe Orton]
+ *) mod_dav: Provide a mechanism to obtain the request_rec and pathname
+ from the dav_resource. [Jari Urpalainen <jari.urpalainen nokia.com>,
+ Brian France <brian brianfrance.com>]
+
*) Build: Use install instead of cp if available on installing
modules to avoid segmentation fault. PR 47951. [hirose31 gmail.com]
Modified: httpd/httpd/trunk/modules/dav/fs/repos.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/fs/repos.c?rev=823703&r1=823702&r2=823703&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
+++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
@@ -46,6 +46,7 @@
apr_pool_t *pool; /* memory storage pool associated with request */
const char *pathname; /* full pathname to resource */
apr_finfo_t finfo; /* filesystem info */
+ request_rec *r;
};
/* private context for doing a filesystem walk */
@@ -210,6 +211,11 @@
**
** PRIVATE REPOSITORY FUNCTIONS
*/
+request_rec *dav_fs_get_request_rec(const dav_resource *resource)
+{
+ return resource->info->r;
+}
+
apr_pool_t *dav_fs_pool(const dav_resource *resource)
{
return resource->info->pool;
@@ -648,6 +654,7 @@
/* Create private resource context descriptor */
ctx = apr_pcalloc(r->pool, sizeof(*ctx));
ctx->finfo = r->finfo;
+ ctx->r = r;
/* ### this should go away */
ctx->pool = r->pool;
@@ -1816,6 +1823,9 @@
dav_fs_remove_resource,
dav_fs_walk,
dav_fs_getetag,
+ dav_fs_get_request_rec,
+ dav_fs_pathname,
+ NULL
};
static dav_prop_insert dav_fs_insert_prop(const dav_resource *resource,
Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.h?rev=823703&r1=823702&r2=823703&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
+++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9 21:41:31 2009
@@ -1940,6 +1940,12 @@
** then this field may be used. In most cases, it will just be NULL.
*/
void *ctx;
+
+ /* return request record */
+ request_rec * (*get_request_rec)(const dav_resource *resource);
+
+ /* return path */
+ const char * (*get_pathname)(const dav_resource *resource);
};
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 12, 2009, at 5:15 PM, Brian J. France wrote:
>
> On Oct 12, 2009, at 4:39 PM, Joe Orton wrote:
>
>> On Mon, Oct 12, 2009 at 04:17:00PM -0400, Brian J. France wrote:
>>> On Oct 12, 2009, at 3:58 AM, Joe Orton wrote:
>>>> On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
>>>>> This creates the following warning:
>>>>>
>>>>> repos.c:1827: warning: initialization from incompatible pointer
>>>>> type
>>>>
>>>> Plus the dav_fs_get_request_rec prototype needs to be moved up.
>>>>
>>>> /local/asf/httpd-trunk/modules/dav/fs/repos.c:214: warning: no
>>>> previous
>>>> prototype for ‘dav_fs_get_request_rec’
>>>
>>> I don't know why that warning is happening. dav_fs_get_request_rec
>>> is
>>> defined on line 214 and used in the struct on line 1827.
>>
>> Sorry, my mistake, it's not a positioning problem - the function
>> needs
>> to be made static.
>
> Updated patch:
Scratch that, here is a patch:
Index: modules/dav/fs/repos.c
===================================================================
--- modules/dav/fs/repos.c (revision 824480)
+++ modules/dav/fs/repos.c (working copy)
@@ -211,7 +211,7 @@
**
** PRIVATE REPOSITORY FUNCTIONS
*/
-request_rec *dav_fs_get_request_rec(const dav_resource *resource)
+static request_rec *dav_fs_get_request_rec(const dav_resource
*resource)
{
return resource->info->r;
}
@@ -1823,9 +1823,9 @@
dav_fs_remove_resource,
dav_fs_walk,
dav_fs_getetag,
+ NULL,
dav_fs_get_request_rec,
- dav_fs_pathname,
- NULL
+ dav_fs_pathname
};
static dav_prop_insert dav_fs_insert_prop(const dav_resource
*resource,
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 12, 2009, at 4:39 PM, Joe Orton wrote:
> On Mon, Oct 12, 2009 at 04:17:00PM -0400, Brian J. France wrote:
>> On Oct 12, 2009, at 3:58 AM, Joe Orton wrote:
>>> On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
>>>> This creates the following warning:
>>>>
>>>> repos.c:1827: warning: initialization from incompatible pointer
>>>> type
>>>
>>> Plus the dav_fs_get_request_rec prototype needs to be moved up.
>>>
>>> /local/asf/httpd-trunk/modules/dav/fs/repos.c:214: warning: no
>>> previous
>>> prototype for ‘dav_fs_get_request_rec’
>>
>> I don't know why that warning is happening. dav_fs_get_request_rec is
>> defined on line 214 and used in the struct on line 1827.
>
> Sorry, my mistake, it's not a positioning problem - the function needs
> to be made static.
Updated patch:
Index: modules/dav/fs/repos.c
===================================================================
--- modules/dav/fs/repos.c (revision 824480)
+++ modules/dav/fs/repos.c (working copy)
@@ -211,7 +211,7 @@
**
** PRIVATE REPOSITORY FUNCTIONS
*/
-request_rec *dav_fs_get_request_rec(const dav_resource *resource)
+status request_rec *dav_fs_get_request_rec(const dav_resource
*resource)
{
return resource->info->r;
}
@@ -1823,9 +1823,9 @@
dav_fs_remove_resource,
dav_fs_walk,
dav_fs_getetag,
+ NULL,
dav_fs_get_request_rec,
- dav_fs_pathname,
- NULL
+ dav_fs_pathname
};
static dav_prop_insert dav_fs_insert_prop(const dav_resource
*resource,
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES
modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Oct 12, 2009 at 04:17:00PM -0400, Brian J. France wrote:
> On Oct 12, 2009, at 3:58 AM, Joe Orton wrote:
>> On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
>>> This creates the following warning:
>>>
>>> repos.c:1827: warning: initialization from incompatible pointer type
>>
>> Plus the dav_fs_get_request_rec prototype needs to be moved up.
>>
>> /local/asf/httpd-trunk/modules/dav/fs/repos.c:214: warning: no
>> previous
>> prototype for ‘dav_fs_get_request_rec’
>
> I don't know why that warning is happening. dav_fs_get_request_rec is
> defined on line 214 and used in the struct on line 1827.
Sorry, my mistake, it's not a positioning problem - the function needs
to be made static.
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/12/2009 10:17 PM, Brian J. France wrote:
>
> On Oct 12, 2009, at 3:58 AM, Joe Orton wrote:
>
>> On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
>>> On 10/09/2009 11:41 PM, minfrin@apache.org wrote:
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> ====================================================================
>>>> --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
>>>> +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
>>>
>>>> @@ -1816,6 +1823,9 @@
>>>> dav_fs_remove_resource,
>>>> dav_fs_walk,
>>>> dav_fs_getetag,
>>>> + dav_fs_get_request_rec,
>>>> + dav_fs_pathname,
>>>> + NULL
>>>
>>> This creates the following warning:
>>>
>>> repos.c:1827: warning: initialization from incompatible pointer type
>>
>> Plus the dav_fs_get_request_rec prototype needs to be moved up.
>>
>> /local/asf/httpd-trunk/modules/dav/fs/repos.c:214: warning: no previous
>> prototype for ‘dav_fs_get_request_rec’
>
>
> I don't know why that warning is happening. dav_fs_get_request_rec is
> defined on line 214 and used in the struct on line 1827.
I guess because there is no prototype defined in repos.h like for dav_fs_pathname.
Furthermore I guess Joe didn't like that *ctx is no longer at the end of the struct.
I assume you did this to stay backportable, but IMHO this cannot be backported
anyway since adding fields to this struct and using them would cause
"old" providers that don't provide them when registering to possibly segfault.
Regards
Rüdiger
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 12, 2009, at 3:58 AM, Joe Orton wrote:
> On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
>> On 10/09/2009 11:41 PM, minfrin@apache.org wrote:
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> ====================================================================
>>> --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
>>> +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31
>>> 2009
>>
>>> @@ -1816,6 +1823,9 @@
>>> dav_fs_remove_resource,
>>> dav_fs_walk,
>>> dav_fs_getetag,
>>> + dav_fs_get_request_rec,
>>> + dav_fs_pathname,
>>> + NULL
>>
>> This creates the following warning:
>>
>> repos.c:1827: warning: initialization from incompatible pointer type
>
> Plus the dav_fs_get_request_rec prototype needs to be moved up.
>
> /local/asf/httpd-trunk/modules/dav/fs/repos.c:214: warning: no
> previous
> prototype for ‘dav_fs_get_request_rec’
I don't know why that warning is happening. dav_fs_get_request_rec is
defined on line 214 and used in the struct on line 1827.
Brian
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES
modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by Joe Orton <jo...@redhat.com>.
On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
> On 10/09/2009 11:41 PM, minfrin@apache.org wrote:
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
> > +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
>
> > @@ -1816,6 +1823,9 @@
> > dav_fs_remove_resource,
> > dav_fs_walk,
> > dav_fs_getetag,
> > + dav_fs_get_request_rec,
> > + dav_fs_pathname,
> > + NULL
>
> This creates the following warning:
>
> repos.c:1827: warning: initialization from incompatible pointer type
Plus the dav_fs_get_request_rec prototype needs to be moved up.
/local/asf/httpd-trunk/modules/dav/fs/repos.c:214: warning: no previous
prototype for ‘dav_fs_get_request_rec’
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 10, 2009, at 4:04 AM, Ruediger Pluem wrote:
> On 10/09/2009 11:41 PM, minfrin@apache.org wrote:
>> Author: minfrin
>> Date: Fri Oct 9 21:41:31 2009
>> New Revision: 823703
>>
>> URL: http://svn.apache.org/viewvc?rev=823703&view=rev
>> Log:
>> mod_dav: Provide a mechanism to obtain the request_rec and pathname
>> from the dav_resource.
>> Submitted by: Jari Urpalainen <jari.urpalainen nokia.com>,
>> Brian France <brian brianfrance.com>
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/dav/fs/repos.c
>> httpd/httpd/trunk/modules/dav/main/mod_dav.h
>>
>
>> Modified: httpd/httpd/trunk/modules/dav/fs/repos.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/fs/repos.c?rev=823703&r1=823702&r2=823703&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
>> +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
>
>> @@ -1816,6 +1823,9 @@
>> dav_fs_remove_resource,
>> dav_fs_walk,
>> dav_fs_getetag,
>> + dav_fs_get_request_rec,
>> + dav_fs_pathname,
>> + NULL
>
> This creates the following warning:
>
> repos.c:1827: warning: initialization from incompatible pointer type
>
> I assume the order of the arguments is wrong and needs to be
>
> NULL,
> dav_fs_get_request_rec,
> dav_fs_pathname
>
> instead. See below in the snipped from mod_dav.h:
>
>
>> Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.h?rev=823703&r1=823702&r2=823703&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
>> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9
>> 21:41:31 2009
>> @@ -1940,6 +1940,12 @@
>> ** then this field may be used. In most cases, it will just be
>> NULL.
>> */
>> void *ctx;
>> +
>> + /* return request record */
>> + request_rec * (*get_request_rec)(const dav_resource *resource);
>> +
>> + /* return path */
>> + const char * (*get_pathname)(const dav_resource *resource);
>> };
Below is a patch to fix the bad struct declaration, sorry about that.
I missed it on the binary compatibility conversion.
Brian
Index: modules/dav/fs/repos.c
===================================================================
--- modules/dav/fs/repos.c (revision 824480)
+++ modules/dav/fs/repos.c (working copy)
@@ -1823,9 +1823,9 @@
dav_fs_remove_resource,
dav_fs_walk,
dav_fs_getetag,
+ NULL,
dav_fs_get_request_rec,
- dav_fs_pathname,
- NULL
+ dav_fs_pathname
};
static dav_prop_insert dav_fs_insert_prop(const dav_resource
*resource,
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/09/2009 11:41 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Fri Oct 9 21:41:31 2009
> New Revision: 823703
>
> URL: http://svn.apache.org/viewvc?rev=823703&view=rev
> Log:
> mod_dav: Provide a mechanism to obtain the request_rec and pathname
> from the dav_resource.
> Submitted by: Jari Urpalainen <jari.urpalainen nokia.com>,
> Brian France <brian brianfrance.com>
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/dav/fs/repos.c
> httpd/httpd/trunk/modules/dav/main/mod_dav.h
>
> Modified: httpd/httpd/trunk/modules/dav/fs/repos.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/fs/repos.c?rev=823703&r1=823702&r2=823703&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
> +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
> @@ -1816,6 +1823,9 @@
> dav_fs_remove_resource,
> dav_fs_walk,
> dav_fs_getetag,
> + dav_fs_get_request_rec,
> + dav_fs_pathname,
> + NULL
This creates the following warning:
repos.c:1827: warning: initialization from incompatible pointer type
I assume the order of the arguments is wrong and needs to be
NULL,
dav_fs_get_request_rec,
dav_fs_pathname
instead. See below in the snipped from mod_dav.h:
> Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.h?rev=823703&r1=823702&r2=823703&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9 21:41:31 2009
> @@ -1940,6 +1940,12 @@
> ** then this field may be used. In most cases, it will just be NULL.
> */
> void *ctx;
> +
> + /* return request record */
> + request_rec * (*get_request_rec)(const dav_resource *resource);
> +
> + /* return path */
> + const char * (*get_pathname)(const dav_resource *resource);
> };
>
Regards
Rüdiger
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Posted by Guenter Knauf <fu...@apache.org>.
Hi Brian,
Brian J. France schrieb:
> Ya, I have been trying to get somebody to commit this fixup patch:
>
> http://www.brianfrance.com/software/apache/dav/fixup.diff
done:
http://svn.apache.org/viewvc?rev=830284&view=rev
> It fixes comments of the new function pointers (so they match others),
> moves the function to static to remove compile time warnings and fixes
> the order in the structure definition.
>
> Can anybody with commit access please review and commit?
well, was not much to review :)
the repos.c 2nd hunk was same as what I already suggested - but thanks
for confirming!
Gün.
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
Ya, I have been trying to get somebody to commit this fixup patch:
http://www.brianfrance.com/software/apache/dav/fixup.diff
It fixes comments of the new function pointers (so they match others),
moves the function to static to remove compile time warnings and fixes
the order in the structure definition.
Can anybody with commit access please review and commit?
Brian
On Oct 27, 2009, at 11:58 AM, Guenter Knauf wrote:
> Hi,
> minfrin@apache.org schrieb:
>> Author: minfrin
>> Date: Fri Oct 9 21:41:31 2009
>> New Revision: 823703
>> URL: http://svn.apache.org/viewvc?rev=823703&view=rev
>> Log:
>> mod_dav: Provide a mechanism to obtain the request_rec and pathname
>> from the dav_resource.
>> Submitted by: Jari Urpalainen <jari.urpalainen nokia.com>,
>> Brian France <brian brianfrance.com>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/dav/fs/repos.c
>> httpd/httpd/trunk/modules/dav/main/mod_dav.h
>> Modified: httpd/httpd/trunk/modules/dav/fs/repos.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/fs/repos.c?rev=823703&r1=823702&r2=823703&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
>> +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
>> @@ -46,6 +46,7 @@
>> apr_pool_t *pool; /* memory storage pool associated with
>> request */
>> const char *pathname; /* full pathname to resource */
>> apr_finfo_t finfo; /* filesystem info */
>> + request_rec *r;
>> };
>> /* private context for doing a filesystem walk */
>> @@ -210,6 +211,11 @@
>> **
>> ** PRIVATE REPOSITORY FUNCTIONS
>> */
>> +request_rec *dav_fs_get_request_rec(const dav_resource *resource)
>> +{
>> + return resource->info->r;
>> +}
>> +
>> apr_pool_t *dav_fs_pool(const dav_resource *resource)
>> {
>> return resource->info->pool;
>> @@ -648,6 +654,7 @@
>> /* Create private resource context descriptor */
>> ctx = apr_pcalloc(r->pool, sizeof(*ctx));
>> ctx->finfo = r->finfo;
>> + ctx->r = r;
>> /* ### this should go away */
>> ctx->pool = r->pool;
>> @@ -1816,6 +1823,9 @@
>> dav_fs_remove_resource,
>> dav_fs_walk,
>> dav_fs_getetag,
>> + dav_fs_get_request_rec,
>> + dav_fs_pathname,
>> + NULL
>> };
>> static dav_prop_insert dav_fs_insert_prop(const dav_resource
>> *resource,
> here seems to be a problem with the order:
> Compiling repos.c
> ### mwccnlm Compiler:
> # File: repos.c
> # ----------------
> # 1827: dav_fs_pathname,
> # Error: ^
> # illegal implicit conversion from 'char * (const struct
> dav_resource *)' to
> # 'struct request_rec * (*)(const struct dav_resource *)'
>
> Errors caused tool to abort.
>
> From what we have in mod_dav.h
> /* Get the entity tag for a resource */
> const char * (*getetag)(const dav_resource *resource);
>
> /*
> ** If a provider needs a context to associate with this hooks
> structure,
> ** then this field may be used. In most cases, it will just be
> NULL.
> */
> void *ctx;
>
> /* return request record */
> request_rec * (*get_request_rec)(const dav_resource *resource);
>
> /* return path */
> const char * (*get_pathname)(const dav_resource *resource);
>
>
> it seems to me that it should be:
> Index: repos.c
> ===================================================================
> --- repos.c (revision 830029)
> +++ repos.c (working copy)
> @@ -1823,9 +1823,9 @@
> dav_fs_remove_resource,
> dav_fs_walk,
> dav_fs_getetag,
> + NULL,
> dav_fs_get_request_rec,
> - dav_fs_pathname,
> - NULL
> + dav_fs_pathname
> };
>
>
> Gün.
>
>
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Posted by Guenter Knauf <fu...@apache.org>.
Hi,
minfrin@apache.org schrieb:
> Author: minfrin
> Date: Fri Oct 9 21:41:31 2009
> New Revision: 823703
>
> URL: http://svn.apache.org/viewvc?rev=823703&view=rev
> Log:
> mod_dav: Provide a mechanism to obtain the request_rec and pathname
> from the dav_resource.
> Submitted by: Jari Urpalainen <jari.urpalainen nokia.com>,
> Brian France <brian brianfrance.com>
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/dav/fs/repos.c
> httpd/httpd/trunk/modules/dav/main/mod_dav.h
>
> Modified: httpd/httpd/trunk/modules/dav/fs/repos.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/fs/repos.c?rev=823703&r1=823702&r2=823703&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/fs/repos.c (original)
> +++ httpd/httpd/trunk/modules/dav/fs/repos.c Fri Oct 9 21:41:31 2009
> @@ -46,6 +46,7 @@
> apr_pool_t *pool; /* memory storage pool associated with request */
> const char *pathname; /* full pathname to resource */
> apr_finfo_t finfo; /* filesystem info */
> + request_rec *r;
> };
>
> /* private context for doing a filesystem walk */
> @@ -210,6 +211,11 @@
> **
> ** PRIVATE REPOSITORY FUNCTIONS
> */
> +request_rec *dav_fs_get_request_rec(const dav_resource *resource)
> +{
> + return resource->info->r;
> +}
> +
> apr_pool_t *dav_fs_pool(const dav_resource *resource)
> {
> return resource->info->pool;
> @@ -648,6 +654,7 @@
> /* Create private resource context descriptor */
> ctx = apr_pcalloc(r->pool, sizeof(*ctx));
> ctx->finfo = r->finfo;
> + ctx->r = r;
>
> /* ### this should go away */
> ctx->pool = r->pool;
> @@ -1816,6 +1823,9 @@
> dav_fs_remove_resource,
> dav_fs_walk,
> dav_fs_getetag,
> + dav_fs_get_request_rec,
> + dav_fs_pathname,
> + NULL
> };
>
> static dav_prop_insert dav_fs_insert_prop(const dav_resource *resource,
>
here seems to be a problem with the order:
Compiling repos.c
### mwccnlm Compiler:
# File: repos.c
# ----------------
# 1827: dav_fs_pathname,
# Error: ^
# illegal implicit conversion from 'char * (const struct dav_resource
*)' to
# 'struct request_rec * (*)(const struct dav_resource *)'
Errors caused tool to abort.
From what we have in mod_dav.h
/* Get the entity tag for a resource */
const char * (*getetag)(const dav_resource *resource);
/*
** If a provider needs a context to associate with this hooks
structure,
** then this field may be used. In most cases, it will just be NULL.
*/
void *ctx;
/* return request record */
request_rec * (*get_request_rec)(const dav_resource *resource);
/* return path */
const char * (*get_pathname)(const dav_resource *resource);
it seems to me that it should be:
Index: repos.c
===================================================================
--- repos.c (revision 830029)
+++ repos.c (working copy)
@@ -1823,9 +1823,9 @@
dav_fs_remove_resource,
dav_fs_walk,
dav_fs_getetag,
+ NULL,
dav_fs_get_request_rec,
- dav_fs_pathname,
- NULL
+ dav_fs_pathname
};
Gün.
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Brian J. France wrote:
>
> Anybody want to get together to talk about design for mod_dav_acl at
> ApacheCon?
/me raises hand
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 16, 2009, at 8:01 AM, Graham Leggett wrote:
> Brian J. France wrote:
>
>> mod_dav_acl would use the filename to validate the acls. Like I
>> said, I
>> don't know if get_pathname is needed or we should just use r-
>> >filename
>> and make sure a mod_dav_fs_db module updated it.
>
> As Joe points out, an ACL could refer to something that wasn't a file,
> such as a subversion repository, or something similar.
>
> It would be better if ACLs could be applied to any URI, not just URIs
> that map to files.
>
> mod_dav_acl mapping to files only seriously limits the usefulness of
> the
> module.
My goal is to create a mod_dav_acl that requires acl providers to do
the real work. Like a mod_dav_acl_fs to do file based acls so it
would need a filename, mod_dav_acl_db could do db acls based on uri,
mod_dav_acl_svn could do uri or svn fs based acls.
I think there is one more patch that is not truly acl related that is
required for all of this (etags stuff) before the acl patch.
Anybody want to get together to talk about design for mod_dav_acl at
ApacheCon?
Brian
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c
modules/dav/main/mod_dav.h
Posted by Graham Leggett <mi...@sharp.fm>.
Brian J. France wrote:
> mod_dav_acl would use the filename to validate the acls. Like I said, I
> don't know if get_pathname is needed or we should just use r->filename
> and make sure a mod_dav_fs_db module updated it.
As Joe points out, an ACL could refer to something that wasn't a file,
such as a subversion repository, or something similar.
It would be better if ACLs could be applied to any URI, not just URIs
that map to files.
mod_dav_acl mapping to files only seriously limits the usefulness of the
module.
Regards,
Graham
--
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES
modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Oct 12, 2009 at 05:14:33PM -0400, Brian J. France wrote:
> mod_dav_acl would use the filename to validate the acls. Like I said, I
> don't know if get_pathname is needed or we should just use r->filename
> and make sure a mod_dav_fs_db module updated it.
Why does mod_dav_acl care about filenames, rather than simply the path
segment of the URI?
The whole point of the mod_dav repository abstraction is to allow the
existence of DAV repositories which are not backed by a filesystem -- so
there is not necessarily any filename corresponding to any given
resource.
Regards, Joe
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 12, 2009, at 4:38 PM, Joe Orton wrote:
> On Mon, Oct 12, 2009 at 04:23:59PM -0400, Brian J. France wrote:
>> On Oct 12, 2009, at 3:57 AM, Joe Orton wrote:
>>> On Fri, Oct 09, 2009 at 09:41:32PM -0000, Graham Leggett wrote:
>>>> --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
>>>> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9
>>>> 21:41:31
>>>> +
>>>> + /* return request record */
>>>> + request_rec * (*get_request_rec)(const dav_resource
>>>> *resource);
>>>> +
>>>> + /* return path */
>>>> + const char * (*get_pathname)(const dav_resource *resource);
>>>> };
>>>>
>>>
>>> What is a "pathname" in this context? A URI path? A filesystem
>>> path?
>>> If the latter, what is get_pathname supposed to do for a non-fs-
>>> backed
>>> repository provider?
>>
>>
>> That I don't know, it could use bogus paths. I haven't gone down the
>> path of creating a new mod_dav_fs module, so I don't know exactly
>> how it
>> would work.
>
> Well, there needs to be some API contract specified so that repos
> backends can implement it.
>
> So: why does the resource abstraction need to be extended to return
> the
> filesystem path? What will mod_dav use it for?
mod_dav_acl would use the filename to validate the acls. Like I said,
I don't know if get_pathname is needed or we should just use r-
>filename and make sure a mod_dav_fs_db module updated it.
Brian
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES
modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Oct 12, 2009 at 04:23:59PM -0400, Brian J. France wrote:
> On Oct 12, 2009, at 3:57 AM, Joe Orton wrote:
>> On Fri, Oct 09, 2009 at 09:41:32PM -0000, Graham Leggett wrote:
>>> --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
>>> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9 21:41:31
>>> +
>>> + /* return request record */
>>> + request_rec * (*get_request_rec)(const dav_resource *resource);
>>> +
>>> + /* return path */
>>> + const char * (*get_pathname)(const dav_resource *resource);
>>> };
>>>
>>
>> What is a "pathname" in this context? A URI path? A filesystem path?
>> If the latter, what is get_pathname supposed to do for a non-fs-backed
>> repository provider?
>
>
> That I don't know, it could use bogus paths. I haven't gone down the
> path of creating a new mod_dav_fs module, so I don't know exactly how it
> would work.
Well, there needs to be some API contract specified so that repos
backends can implement it.
So: why does the resource abstraction need to be extended to return the
filesystem path? What will mod_dav use it for?
Regards, Joe
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by "Brian J. France" <br...@brianfrance.com>.
On Oct 12, 2009, at 3:57 AM, Joe Orton wrote:
> On Fri, Oct 09, 2009 at 09:41:32PM -0000, Graham Leggett wrote:
>> --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
>> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9
>> 21:41:31 2009
>> @@ -1940,6 +1940,12 @@
>> ** then this field may be used. In most cases, it will just be
>> NULL.
>> */
>> void *ctx;
>> +
>> + /* return request record */
>> + request_rec * (*get_request_rec)(const dav_resource *resource);
>> +
>> + /* return path */
>> + const char * (*get_pathname)(const dav_resource *resource);
>> };
>>
>
> What is a "pathname" in this context? A URI path? A filesystem path?
> If the latter, what is get_pathname supposed to do for a non-fs-backed
> repository provider?
That I don't know, it could use bogus paths. I haven't gone down the
path of creating a new mod_dav_fs module, so I don't know exactly how
it would work.
> (Also - compare and contrast how all the rest of the comments in this
> structure are descriptive phrases and start with capital letters and
> everything)
Patch below fixes up the comments for the new functions.
Index: modules/dav/main/mod_dav.h
===================================================================
--- modules/dav/main/mod_dav.h (revision 824480)
+++ modules/dav/main/mod_dav.h (working copy)
@@ -1941,10 +1941,10 @@
*/
void *ctx;
- /* return request record */
+ /* Get the request rec for a resource */
request_rec * (*get_request_rec)(const dav_resource *resource);
- /* return path */
+ /* Get the pathname for a resource */
const char * (*get_pathname)(const dav_resource *resource);
};
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES
modules/dav/fs/repos.c modules/dav/main/mod_dav.h
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Oct 09, 2009 at 09:41:32PM -0000, Graham Leggett wrote:
> --- httpd/httpd/trunk/modules/dav/main/mod_dav.h (original)
> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.h Fri Oct 9 21:41:31 2009
> @@ -1940,6 +1940,12 @@
> ** then this field may be used. In most cases, it will just be NULL.
> */
> void *ctx;
> +
> + /* return request record */
> + request_rec * (*get_request_rec)(const dav_resource *resource);
> +
> + /* return path */
> + const char * (*get_pathname)(const dav_resource *resource);
> };
>
What is a "pathname" in this context? A URI path? A filesystem path?
If the latter, what is get_pathname supposed to do for a non-fs-backed
repository provider?
(Also - compare and contrast how all the rest of the comments in this
structure are descriptive phrases and start with capital letters and
everything)
Regards, Joe