httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] Filter registration.
Date Tue, 25 Jul 2000 00:44:21 GMT
On Mon, Jul 24, 2000 at 05:25:01PM -0700, rbb@covalent.net wrote:
> 
> The two files I am including here do nothing but filter
> registration.  This is a combination of Greg's patch and my patch for
> filter registration only!   I can't stress that enough.  :-)

:-)

> The basic concepts are from Greg's patch.  I will try to outline them
> quickly here, but there are more docs in the files.  I have changed the
> names of most of the types to make a bit more sense to me.  I have a hard
> time calling a filter function a callback, and it was getting annoying to
> type.  :-)

Not a problem.

> IMHO there are two more functions needed, but they are so trivial, I will
> wait to add them.  Those functions simply store data in the space provided
> by the filter when it was registered, and then get the data out.  I may
> add these functions tonight, but I'm going to be busy, and I wanted to get
> this posted before I stop for a few hours.

Yes, please defer for a future patch. Let's keep it small and easy :-)

(and note that ap_add_filter() already allows the caller to specify the
 initial context data)

>...
> If nobody objects I will be committing this code in 48 hours or so.

I'm presuming you mean to commit these files to src/main/ and src/include ?

I have a couple minor isues noted below and then I'm +1

>...
> #include "util_filter.h"
> #include "apr_buf.h"

apr_buf.h and its types aren't part of Apache yet, so we can't include this
right now.

>...
> API_EXPORT(int) ap_pass_brigade(request_rec *r, ap_filter_t *filter, 
>                                  ap_bucket_brigade *bucket)
> {

This function should not be included (yet). We don't have a final design for
passing buckets/brigades, and the ap_bucket_brigade type is not yet visible
to Apache.

>...
> #include "httpd.h"
> #include "apr_buf.h"

Subtract this include.

>...
> typedef int (*ap_filter_func)(request_rec *r, ap_filter_t *filter,
>                                     ap_bucket_brigade *bucket);

Let's have this return an ap_status_t, and also lose the params for now:

typedef ap_status_t (*ap_filter_func)();


This will allow either patch set to use/call the filter function with
whatever params are appropriate for that design. Alternatively, the patch
set can tweak this definition to its proper value (this alternative is
probably "best").

>...
> API_EXPORT(int) ap_pass_brigade(request_rec *r, ap_filter_t *filter, 
>                                  ap_bucket_brigade *bucket);

Please remove this for now.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message