Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Apache: Dev

Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h

 

 

Apache dev RSS feed   Index | Next | Previous | View Threaded


rpluem at apache

Oct 10, 2009, 1:04 AM

Post #1 of 19 (1486 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h

On 10/09/2009 11:41 PM, minfrin [at] apache 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


jorton at redhat

Oct 12, 2009, 12:57 AM

Post #2 of 19 (1452 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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


jorton at redhat

Oct 12, 2009, 12:58 AM

Post #3 of 19 (1448 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

On Sat, Oct 10, 2009 at 10:04:57AM +0200, Ruediger Pluem wrote:
> On 10/09/2009 11:41 PM, minfrin [at] apache 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’


brian at brianfrance

Oct 12, 2009, 1:16 PM

Post #4 of 19 (1446 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

On Oct 10, 2009, at 4:04 AM, Ruediger Pluem wrote:
> On 10/09/2009 11:41 PM, minfrin [at] apache 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,


brian at brianfrance

Oct 12, 2009, 1:17 PM

Post #5 of 19 (1453 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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 [at] apache 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


brian at brianfrance

Oct 12, 2009, 1:23 PM

Post #6 of 19 (1455 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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);
};


rpluem at apache

Oct 12, 2009, 1:32 PM

Post #7 of 19 (1451 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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 [at] apache 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


jorton at redhat

Oct 12, 2009, 1:38 PM

Post #8 of 19 (1443 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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


jorton at redhat

Oct 12, 2009, 1:39 PM

Post #9 of 19 (1443 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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.


brian at brianfrance

Oct 12, 2009, 2:14 PM

Post #10 of 19 (1453 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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


brian at brianfrance

Oct 12, 2009, 2:15 PM

Post #11 of 19 (1454 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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,


brian at brianfrance

Oct 12, 2009, 2:44 PM

Post #12 of 19 (1445 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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,


jorton at redhat

Oct 15, 2009, 7:59 AM

Post #13 of 19 (1390 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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


minfrin at sharp

Oct 16, 2009, 5:01 AM

Post #14 of 19 (1376 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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


brian at brianfrance

Oct 22, 2009, 9:42 AM

Post #15 of 19 (1302 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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


wrowe at rowe-clan

Oct 22, 2009, 5:54 PM

Post #16 of 19 (1300 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

Brian J. France wrote:
>
> Anybody want to get together to talk about design for mod_dav_acl at
> ApacheCon?


/me raises hand


fuankg at apache

Oct 27, 2009, 8:58 AM

Post #17 of 19 (1231 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

Hi,
minfrin [at] apache 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.


brian at brianfrance

Oct 27, 2009, 9:29 AM

Post #18 of 19 (1227 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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 [at] apache 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.
>
>


fuankg at apache

Oct 27, 2009, 11:40 AM

Post #19 of 19 (1229 views)
Permalink
Re: svn commit: r823703 - in /httpd/httpd/trunk: CHANGES modules/dav/fs/repos.c modules/dav/main/mod_dav.h [In reply to]

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.

Apache dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.