You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@covalent.net on 2000/07/25 02:25:01 UTC

[PATCH] Filter registration.

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.  :-)

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.

The basic concepts are simple:

A filter is a function that takes the request_rec for the current request,
the filter chain, and the bucket brigade that has been composed so
far.  Each filter has an opportunity to operate on the current bucket
brigade before it is passed to the next filter in the chain.  The bottom
filter is responsible for writing out to the network.

Filters are inserted into the list in the order in which they wish to be
called.  This means that the core_filter should be inserted into the list
last, so that it is called last.

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

This code can also be found in the repository at:

src/lib/apr/buckets/util_filter.[ch]

Ryan


util_filter.c
=============

/* ====================================================================
 * The Apache Software License, Version 1.1
 *
 * Copyright (c) 2000 The Apache Software Foundation.  All rights
 * reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 *
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 *
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in
 *    the documentation and/or other materials provided with the
 *    distribution.
 *
 * 3. The end-user documentation included with the redistribution,
 *    if any, must include the following acknowledgment:
 *       "This product includes software developed by the
 *        Apache Software Foundation (http://www.apache.org/)."
 *    Alternately, this acknowledgment may appear in the software itself,
 *    if and wherever such third-party acknowledgments normally appear.
 *
 * 4. The names "Apache" and "Apache Software Foundation" must
 *    not be used to endorse or promote products derived from this
 *    software without prior written permission. For written
 *    permission, please contact apache@apache.org.
 *
 * 5. Products derived from this software may not be called "Apache",
 *    nor may "Apache" appear in their name, without prior written
 *    permission of the Apache Software Foundation.
 *
 * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
 * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
 * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
 * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
 * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 * ====================================================================
 *
 * This software consists of voluntary contributions made by many
 * individuals on behalf of the Apache Software Foundation.  For more
 * information on the Apache Software Foundation, please see
 * <http://www.apache.org/>.
 */

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

/*
 * ap_filter_rec_t:
 *
 * This (internal) structure is used for recording information about the
 * registered filters. It associates a name with the filter's callback
 * and filter type.
 *
 * At the moment, these are simply linked in a chain, so a ->next pointer
 * is available.
 */
typedef struct ap_filter_rec_t {
    const char *name;
    ap_filter_func filter_func;
    ap_filter_type ftype;

    struct ap_filter_rec_t *next;
} ap_filter_rec_t;

/* ### make this visible for direct manipulation?
   ### use a hash table
*/
static ap_filter_rec_t *registered_filters = NULL;

/* NOTE: Apache's current design doesn't allow a pool to be passed thu,
   so we depend on a global to hold the correct pool
*/
#define FILTER_POOL     ap_global_hook_pool
#include "ap_hooks.h"   /* for ap_global_hook_pool */

/*
** This macro returns true/false if a given filter should be inserted BEFORE
** another filter. This will happen when one of: 1) there isn't another
** filter; 2) that filter has a higher filter type (class); 3) that filter
** corresponds to a different request.
*/
#define INSERT_BEFORE(f, before_this) ((before_this) == NULL \
                                       || (before_this)->ftype > (f)->ftype)


static ap_status_t filter_cleanup(void *ctx)
{
    registered_filters = NULL;
    return APR_SUCCESS;
}

API_EXPORT(void) ap_register_filter(const char *name,
                                    ap_filter_func filter_func,
                                    ap_filter_type ftype)
{
    ap_filter_rec_t *frec = ap_palloc(FILTER_POOL, sizeof(*frec));

    frec->name = name;
    frec->filter_func = filter_func;
    frec->ftype = ftype;

    frec->next = registered_filters;
    registered_filters = frec;

    ap_register_cleanup(FILTER_POOL, NULL, filter_cleanup, NULL);
}

API_EXPORT(void) ap_add_filter(const char *name, void *ctx, request_rec *r)
{
    ap_filter_rec_t *frec = registered_filters;

    for (; frec != NULL; frec = frec->next) {
        if (!strcasecmp(name, frec->name)) {
            ap_filter_t *f = ap_pcalloc(r->pool, sizeof(*f));

            f->filter_func = frec->filter_func;
            f->ctx = ctx;
            f->ftype = frec->ftype;

            if (INSERT_BEFORE(f, r->filters)) {
                f->next = r->filters;
                r->filters = f;
            }
            else {
                ap_filter_t *fscan = r->filters;
                while (!INSERT_BEFORE(f, fscan->next))
                    fscan = fscan->next;
                f->next = fscan->next;
                fscan->next = f;
            }

            break;
        }
    }
}

API_EXPORT(int) ap_pass_brigade(request_rec *r, ap_filter_t *filter, 
                                 ap_bucket_brigade *bucket)
{
    if (filter == NULL) {
        return APR_ENOTIMPL;
    }
    else {
        return (*filter->filter_func)(r, filter->next, bucket);
    }
}

util_filter.h
=============

/* ====================================================================
 * The Apache Software License, Version 1.1
 *
 * Copyright (c) 2000 The Apache Software Foundation.  All rights
 * reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 *
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 *
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in
 *    the documentation and/or other materials provided with the
 *    distribution.
 *
 * 3. The end-user documentation included with the redistribution,
 *    if any, must include the following acknowledgment:
 *       "This product includes software developed by the
 *        Apache Software Foundation (http://www.apache.org/)."
 *    Alternately, this acknowledgment may appear in the software itself,
 *    if and wherever such third-party acknowledgments normally appear.
 *
 * 4. The names "Apache" and "Apache Software Foundation" must
 *    not be used to endorse or promote products derived from this
 *    software without prior written permission. For written
 *    permission, please contact apache@apache.org.
 *
 * 5. Products derived from this software may not be called "Apache",
 *    nor may "Apache" appear in their name, without prior written
 *    permission of the Apache Software Foundation.
 *
 * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
 * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
 * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
 * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
 * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 * ====================================================================
 *
 * This software consists of voluntary contributions made by many
 * individuals on behalf of the Apache Software Foundation.  For more
 * information on the Apache Software Foundation, please see
 * <http://www.apache.org/>.
 */

#ifndef AP_FILTER_H
#define AP_FILTER_H

#ifdef __cplusplus
extern "C" {
#endif

#ifdef APR_HAVE_STDARG_H
#include <stdarg.h>
#endif

#include "httpd.h"
#include "apr_buf.h"
#include "apr.h"

/*
 * FILTER CHAIN
 *
 * Filters operate using a "chaining" mechanism. The filters are chained
 * together into a sequence. When output is generated, it is passed through
 * each of the filters on this chain, until it reaches the end (or "bottom")
 * and is placed onto the network.
 *
 * The top of the chain, the code generating the output, is typically called
 * a "content generator." The content generator's output is fed into the
 * filter chain using the standard Apache output mechanisms: ap_rputs(),
 * ap_rprintf(), ap_rwrite(), etc.
 *
 * Each filter is defined by a callback. This callback takes the output from
 * the previous filter (or the content generator if there is no previous
 * filter), operates on it, and passes the result to the next filter in the
 * chain. This pass-off is performed using the ap_fc_* functions, such as
 * ap_fc_puts(), ap_fc_printf(), ap_fc_write(), etc.
 *
 * When content generation is complete, the system will pass an "end of
 * stream" marker into the filter chain. The filters will use this to flush
 * out any internal state and to detect incomplete syntax (for example, an
 * unterminated SSI directive).
 */

/* forward declare the filter type */
typedef struct ap_filter_t ap_filter_t;

/*
 * ap_filter_func:
 *
 * This function type is used for filter callbacks. It will be passed a
 * pointer to "this" filter, and a "bucket" containing the content to be
 * filtered.
 *
 * In filter->ctx, the callback will find its context. This context is
 * provided here, so that a filter may be installed multiple times, each
 * receiving its own per-install context pointer.
 *
 * Callbacks are associated with a filter definition, which is specified
 * by name. See ap_register_filter() for setting the association between
 * a name for a filter and its associated callback (and other information).
 *
 * The *bucket structure (and all those referenced by ->next and ->prev)
 * should be considered "const". The filter is allowed to modify the
 * next/prev to insert/remove/replace elements in the bucket list, but
 * the types and values of the individual buckets should not be altered.
 */
typedef int (*ap_filter_func)(request_rec *r, ap_filter_t *filter,
                                    ap_bucket_brigade *bucket);

/*
 * ap_filter_type:
 *
 * Filters have different types/classifications. These are used to group
 * and sort the filters to properly sequence their operation.
 *
 * AP_FTYPE_CONTENT:
 *     These filters are used to alter the content that is passed through
 *     them. Examples are SSI or PHP.
 *
 * AP_FTYPE_CONNECTION:
 *     These filters will alter the content, but in ways that are more
 *     strongly associated with the output connection. Examples are
 *     compression, character recoding, or chunked transfer coding.
 *
 *     It is important to note that these types of filters are not allowed
 *     in a sub-request. A sub-requests output can certainly be filtered
 *     by AP_FTYPE_CONTENT filters, but all of the "final processing" is
 *     determined by the main request.
 *
 * The types have a particular sort order, which allows us to insert them
 * into the filter chain in a determistic order. Within a particular grouping,
 * the ordering is equivalent to the order of calls to ap_add_filter().
 */
typedef enum {
    AP_FTYPE_CONTENT,
    AP_FTYPE_CONNECTION
} ap_filter_type;

/*
 * ap_filter_t:
 *
 * This is the request-time context structure for an installed filter (in
 * the output filter chain). It provides the callback to use for filtering,
 * the request this filter is associated with (which is important when
 * an output chain also includes sub-request filters), the context for this
 * installed filter, and the filter ordering/chaining fields.
 *
 * Filter callbacks are free to use ->ctx as they please, to store context
 * during the filter process. Generally, this is superior over associating
 * the state directly with the request. A callback should not change any of
 * the other fields.
 */
struct ap_filter_t {
    ap_filter_func filter_func;

    void *ctx;

    ap_filter_type ftype;
    ap_filter_t *next;
};

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


/*
 * ap_register_filter():
 *
 * This function is used to register a filter with the system. After this
 * registration is performed, then a filter may be added into the filter
 * chain by using ap_add_filter() and simply specifying the name.
 *
 * The filter's callback and type should be passed.
 */
API_EXPORT(void) ap_register_filter(const char *name,
                                    ap_filter_func filter_func,
                                    ap_filter_type ftype);

/*
 * ap_add_filter():
 *
 * Adds a named filter into the filter chain on the specified request record.
 * The filter will be installed with the specified context pointer.
 *
 * Filters added in this way will always be placed at the end of the filters
 * that have the same type (thus, the filters have the same order as the
 * calls to ap_add_filter). If the current filter chain contains filters
 * from another request, then this filter will be added before those other
 * filters.
 */
API_EXPORT(void) ap_add_filter(const char *name, void *ctx, request_rec *r);


#ifdef __cplusplus
}
#endif

#endif	/* !AP_FILTER_H */


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: Not [PATCH] Filter registration :)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 24, 2000 at 08:55:54PM -0500, William A. Rowe, Jr. wrote:
> And you suggest there are no issues with filtering + async (LOL)

Never said that. I said that it is inappropriate to use async as a reason
for preventing filtering work. Async will have an impact on *everything*.
Since we must rewrite everything to deal with async, then any filtering we
do now just falls into that "rewrite it all" work item. And while we may be
able to fine tune a bit of the filtering for the async model, none of the
designs on the table have a great match for a "pure" async implementation.
IOW, we will end up with a hybrid async/sync model.

And no: I'm not up for discussing async now. We aren't go to do it any time
soon, so why spend time talking about it now?

> > From: Greg Stein [mailto:gstein@lyra.org]
> > Sent: Monday, July 24, 2000 8:47 PM
> >
> > If a bucket has a self-defined lifetime, then the ap_bucket_t must be
> > allocated from the heap/pool. Usually, its data must also be managed in a
> > way that is separate from the execution stack (e.g. rmem buckets have issues
> > in that they can refer to stack-based data; that prevents their self-
> > defined lifetime).
> 
> I actually found your #3 - pipe read is a terminal condition that can't be
> undone, to be the most significant issue.
> 
> By definition, when and if we transition to async buckets, nothing can be
> living on the stack.

"... living on the stack [at the time you pass the buckets to the async
output manager]." In other words, you copy the data off the stack at that
point.

Some background for terminology:

    {
        char buf[100];
	int n;
	
	n = fill_buf_with_stuff(buf, sizeof(buf));
	ap_rwrite(buf, n, r);
    }

Let's refer to the data residing in "buf" as "stack-based data".

If the stack-based data is going to be translated, then there will never be
a need to copy it (the original data is never passed, just the translated
data). If the stack-based data needs to live past the function return, then
it must be copied. So... to possibly optimize the number of copies, it is
important to allow stack-based data to exist within the output chain. At the
point that the lifetime of that data needs to change (if ever!), then it can
be copied.

[ there is an argument that we should never pass stack-based data into the
  filter chain. I believe that is a poor decision, but I'm hoping that I
  don't have to illustrate why... ]

> Not that we can't pass the bucket from filter to
> filter as arguments, but it needs to live in the pool.  And, as far as 
> the lifetime is concerned, I suggest they live for the duration of their 
> request, unless discarded (read on :)

Actually, the data would need to live for the duration of the *connection*.
It will be nice to jam two responses into one network packet. Thus, the data
going into that network packet must survive the finalization of the first
request.

> Is there any thought to fixed-size buckets (8k, for example) in a common
> pool for the process (talking about a threaded model, here, or perhaps
> in shared memory across processes) that will live for eternity?  Simply 
> grab and discard as the filters pass them around.  If I must rewrite a 
> bucket (can't just tweak it), then grab another bucket and throw the 
> last back to the pool.  If I need a second bucket (growing the response), 
> then I need to just grab one from the pool.  All buckets must be thrown 
> back into the available pool by Apache at the end of the request.

Nope. No thoughts have been applied in this area. You are effectively
describing pools -- pools keep a "free list" of blocks to use for future
allocations. Of course, how this interacts with pools... dunno. I'm not sure
what the design's goal is.

> I don't like relying on the module writer to do-it-right when we will be
> dying a slow, painful death due to a module's leak.

Any time you allocate, it is possible that you may leak. We avoid those
through the use of pools. If your bucket structures (not necessarily the
data!) are defined on the stack, then you can't possibly leak them. The
data referenced by the bucket has the same lifetime, so it is also quite
easy to manage it.

It only gets complicated when somebody wants to "set aside" data. The other
complication is at the bottom of the filter chain. What is the "right"
mechanism for buffering up network output to minimize the number of packets?

> The advantage lies in the fact that shared file memory can be swapped in
> and out, the bucket list can grow if necessary, and could even be shrunk
> if we always allocate from the head (so once that ornery huge request is
> done, and odd leftover buckets are released, noone is sitting at the end
> of the pool.)
> 
> Thoughts?

I don't quite follow your description. I don't quite see the benefits over
using pools for the memory management. Care to code up something to
demonstrate the idea? :-)

Cheers,
-g

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

Re: [PATCH] Filter registration.

Posted by Tony Finch <do...@dotat.at>.
"Roy T. Fielding" <fi...@kiwi.ICS.UCI.EDU> wrote:
>
>We just need to be on top of it for the interface, as is the case in
>your design, though I'd hope we could lump them into more general
>categories than by implementation name.

Hmm, yes, but I couldn't think of a way to break down the space into
orthogonal dimensions that simplify the whole thing. Perhaps each
colour's methods take up too little space, or they don't overlap very
much.

>Anyway, I just wanted to be sure that I answered your question -- I really
>do need to work on the dissertation now.

Yes, you do :-)

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
463 heart-shaped patch on the quilt of inappropriateness

Re: [PATCH] Filter registration.

Posted by Tony Finch <do...@dotat.at>.
"Roy T. Fielding" <fi...@kiwi.ICS.UCI.EDU> wrote:
>
>>But then the data has changed too, so you still have the situation
>>that the lifetime of the bucket is the same as its contents. The same
>>is true for the bottom filter if it concatenates the buckets when it
>>saves the contents: the old data and buckets are destroyed and
>>replaced by new ones. I think that if you keep the bucket and content
>>lifetimes the same then it will simplify the rules you have to follow
>>to ensure that buckets are used correctly.
>
>The data doesn't change -- it gets copied to other places, or new
>references to it are created.

Yes, that's what I meant. The new bucket refers to different data from
the old bucket.

>E.g., when we truncate a bucket, all we are doing is reducing the
>length size within the bucket structure.

Yes, this is why things like HEAP buckets and MMAP buckets have to
have a reference counting mechanism so that the underlying resource is
freed when all of its buckets disappear. However, when you don't want
to leave resource management to the bucket code you use IMMORTAL
buckets which mean "lay off, this is my data".

>I'm not sure how far we can go with that, since some data sources can
>only be "read" from a single point.

Are you referring to PIPEs here? I described a way of dealing with
them inside this design.

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
297 the humor dial's lowest setting

Re: itanium builds

Posted by Mark J Cox <ma...@awe.com>.
> has anyone taken advantage of sourceforge's itanium boxes to try compiling
> apache?

We've got a 4-way IA-64 box here running Red Hat Linux that works with
Apache just fine.  There was a couple of problems getting PHP running on
it, but we sent a fix to Rasmus.  Most things like mod_ssl, OpenSSL, Perl
and so on work fine too (although OpenSSL isn't optimised yet)

Mark
Mark J Cox, .......................................... www.awe.com/mark
Apache Software Foundation ..... OpenSSL Group ..... Apache Week editor






Re: itanium builds

Posted by Bill Stoddard <re...@attglobal.net>.
Paul Reder has an Apache port running on a version of AIX that runs on itanium hardware (project
Monterey, if that means anything to you).  Wasn't a very difficult port to accomodate (not exploit)
the 64 bit hardware. Will be checking in the port when Monterey becomes available.  I suspect a
Linux port would not be substantially different.

Bill

> On Fri, 28 Jul 2000, blue wrote:
>
> > has anyone taken advantage of sourceforge's itanium boxes to try compiling
> > apache?
>
> I haven't seen anyone answering this, or referring to any work they are
> doing on this front - any objections to me doing so??
>
>
> James.
>
>


Re: itanium builds

Posted by James Sutherland <ja...@cam.ac.uk>.
On Fri, 28 Jul 2000, blue wrote:

> has anyone taken advantage of sourceforge's itanium boxes to try compiling
> apache?

I haven't seen anyone answering this, or referring to any work they are
doing on this front - any objections to me doing so??


James.


itanium builds

Posted by blue <bl...@calico.gator.net>.
has anyone taken advantage of sourceforge's itanium boxes to try compiling
apache?

-- 
        Blue Lang                              Unix Systems Admin
        QSP, Inc., 3200 Atlantic Ave, Ste 100, Raleigh, NC, 27604
        Home: 919 835 1540  Work: 919 875 6994  Fax: 919 872 4015



Re: [PATCH] Filter registration.

Posted by Tony Finch <do...@dotat.at>.
"Roy T. Fielding" <fi...@kiwi.ICS.UCI.EDU> wrote:
>
>>I don't think this is useful because a lot of the data that you put
>>into a bucket has a longer lifetime than only one trip down and up the
>>call stack, and you want to be able to make the most of that property.
>
>That is the data inside the bucket.  The bucket structure itself is
>rarely usable across invocations because of the way filters work -- they
>end up either transforming the data (in which case the bucket goes away)
>or storing the data in a way that is efficient for later processing
>(in which case the bucket will most likely go away).

But then the data has changed too, so you still have the situation
that the lifetime of the bucket is the same as its contents. The same
is true for the bottom filter if it concatenates the buckets when it
saves the contents: the old data and buckets are destroyed and
replaced by new ones. I think that if you keep the bucket and content
lifetimes the same then it will simplify the rules you have to follow
to ensure that buckets are used correctly.

>In some cases it is best to allocate the bucket structure from a
>heap, and in others it is best to allocate it from the stack.  We could
>easily choose either one (provided the allocations come from the right
>pool in the heap case, and provided that the filter calls downstream
>filters on a write for the stack case).  Greg's point, I believe, is
>that it is easier to manage and more efficient on the common case
>for the bucket structure to always be allocated from the stack, or at
>least to assume that is the case within receiving filters.  We then only
>have to worry about managing the lifetime of the data within the bucket.
>Try it and see. 

I'm scared that playing mix-and-match with allocators is a recipe for
confusion and bugs. Perhaps it can be solved by having special bucket
colours for buckets in different places... But I know that allocating
only on the stack is too limiting, and that some of the unpleasant
complexity of C++ is due to the fact that it tries to make objects on
the stack work as well as objects on the heap... Dunno. Let's try it
and see :-)

>It's an interesting idea, but all you are doing is naming the function
>that is set as the free() pointer for the bucket.  Rather than coloring
>buckets by their behavior, we should be coloring them by their interface.
>That is, the significant distinctions between bucket types are:
>
>   how do you read the data within this bucket
>   what are you supposed to do with the data read (metadata vs data)
>   how do you set aside this bucket for the next call

I think that's exactly what I was trying to describe. The paragraphs
you snipped explained the different behaviours of the bucket methods
for the interesting colours, i.e. read and consume as well as free
(which is just a part of the consume method that runs when all the
data has been consumed).

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
464 whipstitch ligament fix

Re: [PATCH] Filter registration.

Posted by Tony Finch <do...@dotat.at>.
Tony Finch <do...@dotat.at> wrote:
>
>IMMORTAL
>	data that's around for ever, like literal strings
>MMAP
>	mmap()ed data that must be munmap()ed when the bucket is
>	destroyed

A point I forgot that can be a source of confusion: IMMORTAL buckets
are immortal from the point of view of the filtering code, but from
the point of view of the code that actually owns the data it may not
be immortal at all. E.g. mod_mmap_static keeps a collection of
mmap()ed files around which it hands to the filtering code as IMMORTAL
buckets because it wants to keep responsibility for the data and
doesn't want the filtering gode to munmap() the files. So not all
mmap()ed data is in MMAP buckets, and the same could be true for data
on the heap etc.

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
309 pigeon pudding in your propellor

Re: [PATCH] Filter registration.

Posted by Tony Finch <do...@dotat.at>.
[apologies for the length]

This is the result of conversations with Ryan over the last couple of
days and a few hours of refining the ideas and presentation... It
slightly escaped from me, so I hope it makes sense :-)

"Roy T. Fielding" <fi...@kiwi.ICS.UCI.EDU> wrote:
>Tony Finch <do...@dotat.at> wrote:
>>
>>I think the lifetime of a bucket should be independent of the request
>>handler function because otherwise you lose a lot of advantages of the
>>abstraction. I wouldn't consider it to be a properly first-class data
>>type if you can't return it from a function.
>
>Hmmm, I disagree -- you just have to make it part of the abstraction.
>If we say that the bucket always has to be emptied before it can be
>returned, and we think of the write call always "returning" the
>bucket when it returns, then this does match the abstraction.

I don't think this is useful because a lot of the data that you put
into a bucket has a longer lifetime than only one trip down and up the
call stack, and you want to be able to make the most of that property.

>It also makes a certain degree of sense.  What we really care about
>being persistent is the contents of the bucket, not the bucket itself.

But the bucket exists to hold the metadata for its contents, and so
the bucket should exist as long as the contents do. I'm not very happy
with the current names for the bucket colours because I find them
confusing. Instead I prefer these names; some of them are less
necessary than others so won't be implemented right away:

IMMORTAL
	data that's around for ever, like literal strings
TRANSIENT
	short-lifetime data (e.g. in a buffer that will be re-written
	when the call stack is unwound)
POOL
	data in a pool, e.g. a variable in the environment table in
	the request pool
HEAP
	data on the C heap that must be free()d when the bucket is
	destroyed
MMAP
	mmap()ed data that must be munmap()ed when the bucket is
	destroyed
FILE
	a region of a file (not in memory yet) for sendfile()
PIPE
	data that will (in the future) come from a pipe e.g. a CGI

The basic idea is that when a bucket is created the contents that you
pass to the creation function become the responsibility of the bucket.
There are a couple of exceptions (the immortal and transient colours)
for reasons of optimisation (and because the free operation is a
no-op), and in the pool case the bucket shares responsibility with the
pool code.

Some of the buckets have to be slightly more elaborate in order to
handle the split operation efficiently and to ensure that the data
isn't freed when a bucket still refers to it. I.e. the buckets refer
to an extra structure for holding the data together with a reference
count.

Buckets are consumed in two stages: first the contents (in the form of
a base pointer and length) are retrieved, and then the bucket is told
how many bytes have actually been used (sent to the network or the
next filter) which updates its internal base and length. When all the
bytes have been consumed the bucket is deleted.

Deleting is straightforward except for POOL buckets. Pool buckets
exist so that if their data needs to be kept longer than the pool
(e.g. concatenating the end of one response onto the start of the next
pipelined response) then this can be spotted and the data copied
somewhere safe. If the pool disappears before the bucket does then the
cleanup that the bucket has registered copies the data onto the heap
and turns the bucket into a HEAP bucket. If the opposite occurs the
cleanup just has to be deregistered.

Getting the contents of the bucket is more interesting. Most of the
time the function can just return the base pointer and length that are
held in the bucket, but in the FILE and PIPE cases the pointer and
length aren't there yet. Therefore a FILE bucket would mmap the
required region and morph into an MMAP bucket (which could be done
in-place but it might be simpler to create a new bucket and delete the
old one). A PIPE bucket would read some data from the pipe and put it
into a HEAP bucket which gets inserted into the brigade before the
PIPE bucket which remains there in order to represent the rest of the
stream. (PIPE buckets are cool because they allow you to offload
simple CGIs to a select() thread in the same way that you can with
FILE buckets.)

Another important thing is the handling of TRANSIENT buckets, where
the bottom level filter must copy the data if it needs to stay around
after the function returns. I tend to think this should be handled as
a special-case which would morph TRANSIENT buckets into HEAP buckets,
since other colours don't need to do anything clever.

These bucket types are quite ad-hoc which doesn't seem very elegant,
but splitting the ideas of the lifetime and the type of the data in
the bucket into orthogonal dimensions doesn't provide any obvious
simplifications as far as I can see. In most cases the buckets and
their contents have identical lifetimes, and where this isn't the case
it is to support special semantics or optimisations so I suppose this
makes sense.

If a bucket's contents can be altered in place then there are some
opportunities for optimisation. One possibility is 8 bit character set
translation, but Ryan tells me that the iconv stuff might not work in
place. Another is appending to the end of a bucket brigade with
something like printf(), which happens quite frequently. One option is
to format the data into a buffer on the stack and pass it down as a
TRANSIENT bucket; this is quite efficient if the bucket doesn't have
to be kept (and the data copied). Alternatively, if HEAP bucket data
is overwritable and they are allocated with extra space for data then
you can sprintf() straight into the extra space and keep top
efficiency.

>Consider, for example, real-life bucket brigades.  The bucket is
>passed downstream with water and then back up empty.

This analogy doesn't really work because we don't always completely
empty the buckets at the bottom.

>It is crucial to control the lifetime of the data within the bucket,
>not the bucket structure itself.  The reason is that, on those occasions
>when it is necessary to set bucket data aside, it is often more efficient
>for the server to combine all the buckets into a single page at that
>point rather than set aside both the data and the bucket structures.

Agreed, but that is an optimisation. At this point you are discarding
the original bucket and creating a new one, and you have moved the
data to a new place with a different lifetime.

>Personally, I'd rather reinvent lists than import a huge pile of
>poorly named macros, of which we will only use five or six.  Dean's
>splim has the next/prev pointers in the structure.

*shrug* This is an uninteresting discussion; I was simply trying to
reduce NIH.

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
305 strip-mined comedy ore

Re: [PATCH] Filter registration.

Posted by Tony Finch <do...@dotat.at>.
Greg Stein <gs...@lyra.org> wrote:
>
>1) what is the lifetime of an ap_bucket_t (and/or a list/brigade of them)
>   specifically: are they self-managed or do they die when the filter
>   function returns? this also involves things like the use of malloc()

I think the lifetime of a bucket should be independent of the request
handler function because otherwise you lose a lot of advantages of the
abstraction. I wouldn't consider it to be a properly first-class data
type if you can't return it from a function.

Being able to control the livetime of a bucket brigade independently
of the flow of execution is crucial for allowing advanced stuff like
cacheing and event-driven IO to benefit from the bucket brigade
abstraction. I think it is short-sighted to limit it to nothing more
than what we need now.

>2) do we include a tail pointer? is this tail pointer in a separate
>   structure or is it part of the actual bucket somehow? (e.g. the first
>   bucket's prev pointer refers to the tail)

IMO it is best to keep the distinction between the list (i.e. the bucket
brigade, where the head and tail pointers live etc.) and the list
elements. I also think it would be best to use the standard BSD list
macros rather than waste time implementing lists all over again.

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
436 mysterious mound on your mantelpiece

Not [PATCH] Filter registration :)

Posted by "William A. Rowe, Jr." <wr...@lnd.com>.
And you suggest there are no issues with filtering + async (LOL)

> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Monday, July 24, 2000 8:47 PM
>
> If a bucket has a self-defined lifetime, then the ap_bucket_t must be
> allocated from the heap/pool. Usually, its data must also be managed in a
> way that is separate from the execution stack (e.g. rmem buckets have issues
> in that they can refer to stack-based data; that prevents their self-
> defined lifetime).

I actually found your #3 - pipe read is a terminal condition that can't be
undone, to be the most significant issue.

By definition, when and if we transition to async buckets, nothing can be
living on the stack.  Not that we can't pass the bucket from filter to
filter as arguments, but it needs to live in the pool.  And, as far as 
the lifetime is concerned, I suggest they live for the duration of their 
request, unless discarded (read on :)

Is there any thought to fixed-size buckets (8k, for example) in a common
pool for the process (talking about a threaded model, here, or perhaps
in shared memory across processes) that will live for eternity?  Simply 
grab and discard as the filters pass them around.  If I must rewrite a 
bucket (can't just tweak it), then grab another bucket and throw the 
last back to the pool.  If I need a second bucket (growing the response), 
then I need to just grab one from the pool.  All buckets must be thrown 
back into the available pool by Apache at the end of the request.

I don't like relying on the module writer to do-it-right when we will be
dying a slow, painful death due to a module's leak.

The advantage lies in the fact that shared file memory can be swapped in
and out, the bucket list can grow if necessary, and could even be shrunk
if we always allocate from the head (so once that ornery huge request is
done, and odd leftover buckets are released, noone is sitting at the end
of the pool.)

Thoughts?

Bill


Re: [PATCH] Filter registration.

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 24, 2000 at 08:18:16PM -0500, William A. Rowe, Jr. wrote:
> to both Greg and Ryan:
> 
> > From: Greg Stein [mailto:gstein@lyra.org]
>...
> > This is actually a very cool patch and it allows us to begin 
> > to do some "real" work with the filtering code.
> [snip]
> > In a word: Coolness
> 
> You have -no- idea how glad we new-httpd-watchers are for you two to
> have worked out a patch that you can both accept :-)

hehe...

> I know you have both looked to the list for significant feedback on
> your respective designs, and we are happy to oblige.  Seeing that you
> both feel you -need- specific issues handled by filtering, though,
> you two are absolutely in the best position to argue the merits and
> come to a common perspective (not to slight Dean, Roy, or other key
> architects of Apache!!!)  But when you two can dovetail into an
> acceptable solution, we have the battle won :-)

Well, this is a step towards filtering. Ryan and I spoke on the phone last
night (we missed each other at the conference) and talked through a number
of items about filtering.

[ yes, we spoke :-) ... it isn't like I've got a letter bomb on the way to
  his house or anything... really. honestly. you can trust me. ]


While this is a nice step, there are still a few outstanding issues about
filter models that we haven't quite resolved yet:

1) what is the lifetime of an ap_bucket_t (and/or a list/brigade of them)
   specifically: are they self-managed or do they die when the filter
   function returns? this also involves things like the use of malloc()

2) do we include a tail pointer? is this tail pointer in a separate
   structure or is it part of the actual bucket somehow? (e.g. the first
   bucket's prev pointer refers to the tail)

3) how do we read chunks of data out of a list of buckets? for some buckets
   this will implicitly "damage" the bucket (e.g. a bucket may refer to a
   pipe; once you read data, you can't stick it back into the bucket; you
   must build a new bucket to hold any pipe data you wish to keep) Part of
   this issue relates to Roy's concern about losing information on the
   origination of the data (does the ptr/len refer to a shared working buf
   or can you construct new buckets referring to the data)

There are probably a couple other issues, but I think the real stickler is
issue #1. That issue pretty much defines the difference between the filter
designs that Ryan and I have proposed. The rest (e.g. the recent patch for
filter registration) is basically window dressing.

If a bucket has a self-defined lifetime, then the ap_bucket_t must be
allocated from the heap/pool. Usually, its data must also be managed in a
way that is separate from the execution stack (e.g. rmem buckets have issues
in that they can refer to stack-based data; that prevents their self-
defined lifetime). These types of buckets can be easily set-aside, but they
can also create a situation of "copying earlier rather than later." I tend
to believe that they're harder to work with because you need to explicitly
manage their lifetime. The presence of rmem buckets means that a function is
always needed to "convert" the bucket into something that can be set aside.
Some buckets are no-ops (rwmem is the only one so far), while others may
need to copy rmem data, or "dup" a file handle. If the filter function is
always going to call a "convert" function, I would prefer to see the
execution-based lifetime (which can alter the buckets' lifetime at that
point).

In any case... these final few issues will be handled a bit separately from
the registration issue emboded in this recent patch.

[ note that I'm out of town for a week; Ryan said he's busy, too; the rest
  of the stuff will be picked up next week. ]


Cheers,
-g

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

RE: [PATCH] Filter registration.

Posted by "William A. Rowe, Jr." <wr...@lnd.com>.
to both Greg and Ryan:

> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Monday, July 24, 2000 7:53 PM
> 
> On Mon, Jul 24, 2000 at 05:44:21PM -0700, Greg Stein wrote:
> > 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.  :-)
> > 
> > :-)
> 
> 
> This is actually a very cool patch and it allows us to begin 
> to do some "real" work with the filtering code.
[snip]
> In a word: Coolness

You have -no- idea how glad we new-httpd-watchers are for you two to
have worked out a patch that you can both accept :-)

I know you have both looked to the list for significant feedback on
your respective designs, and we are happy to oblige.  Seeing that you
both feel you -need- specific issues handled by filtering, though,
you two are absolutely in the best position to argue the merits and
come to a common perspective (not to slight Dean, Roy, or other key
architects of Apache!!!)  But when you two can dovetail into an
acceptable solution, we have the battle won :-)

I've tried to follow, and accidently committed some new mod_isapi
code (does very little yet), and intend to write the ISAPI 5.0 spec.
Why?

Until we have an existing demand for filters (a well defined and very
commonly used API), we can't see how they do/don't suit us.  And I got
lost in the debate (agreed we need a bucket with a length, and never a
strlen'ed char*, etc... but those are the simple questions.)

So I'll be patching up the bit of mod_isapi code I broke and introduce
the filtering.  Forewarned lurkers:  an ISAPI filter isn't what we are
thinking of as filters (although they are a part of the parcel.)  ISAPI
filters include such things as header rewrites, redirections, etc, that
don't actually use 'filtering'.

Good luck to you both in solving this code, and ++1 to the patch.

Bill




Re: [PATCH] Filter registration.

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 24, 2000 at 05:44:21PM -0700, Greg Stein wrote:
> 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.  :-)
> 
> :-)


I just wanted to talk about the patch a bit (separate from a few tweaks).

This is actually a very cool patch and it allows us to begin to do some
"real" work with the filtering code. Once we have filter registration and
insertion, then we can begin to implement things like the SetFilter
directive. We can also start to experiment with type-based insertion
(although it seems that people have some concerns about that style).

Basically: this patch can unblock certain types of progressin/development of
filters [independent of the final form of how content is passed through the
chain]

In a word: Coolness

Cheers,
-g

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

Re: [PATCH] Filter registration.

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 24, 2000 at 08:38:51PM -0700, rbb@covalent.net wrote:
> I wrote:
>...
> > Specifically: the ap_filter_t contains the request_rec that the filter was
> > inserted on. This is important if you chain together filters from multiple
> > (sub)requests. The change in filter->r is the signal for the partitioning of
> > filters between (sub)requests.
> > 
> > Roy seems to argue that ->r shouldn't be present anywhere in there. I think
> > that position is too idealistic, so I'm not listening to it :-)  No,
> > seriously, if we *can* get to a point where the request_rec isn't needed in
> > a filter, then it will be easier to remove the sucker from the structure
> > than from the signatures. [because (theoretically) nobody is referring to
> > ->r any more]
> > 
> > But as I mentioned in the other note, we can simply tweak these typedefs and
> > signatures with our specific "filter system" patches.
> 
> BTW, I should point out that ap_filter_t does not contain a request_rec
> right now.  :-)

Have no fear. I saw that :-)

I'm just going to rebuild my patch to insert the thing in there. I
considered its absence from ap_filter_t to be a nit that can be handled in a
followup. The *key* part was the registration design/mechanism and the
absence of ->r had no bearing on that.

[ well, that rebuild won't happen until I get back, but that was my idea ]


Cheers,
-g

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

Re: [PATCH] Filter registration.

Posted by rb...@covalent.net.
> I see your point. However, a placeholder type for a bucket isn't quite
> "enough" as I'd rather see a signature that looks like:
> 
> API_EXPORT(ap_status_t) ap_pass_brigade(ap_filter_t *filter,
>                                         ap_bucket_brigade *bucket)
> 
> Specifically: the ap_filter_t contains the request_rec that the filter was
> inserted on. This is important if you chain together filters from multiple
> (sub)requests. The change in filter->r is the signal for the partitioning of
> filters between (sub)requests.
> 
> Roy seems to argue that ->r shouldn't be present anywhere in there. I think
> that position is too idealistic, so I'm not listening to it :-)  No,
> seriously, if we *can* get to a point where the request_rec isn't needed in
> a filter, then it will be easier to remove the sucker from the structure
> than from the signatures. [because (theoretically) nobody is referring to
> ->r any more]
> 
> But as I mentioned in the other note, we can simply tweak these typedefs and
> signatures with our specific "filter system" patches.

BTW, I should point out that ap_filter_t does not contain a request_rec
right now.  :-)

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: [PATCH] Filter registration.

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Jul 24, 2000 at 08:16:55PM -0700, rbb@covalent.net wrote:
>...
> > 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.
> 
> I know.  I included those in this patch, because we require some kind of
> ap_bucket type.  Knowing that, and thinking that apr_buf or ap_buf is a
> likely name, I decided to add this.

buf? I thought we were talking about buckets? :-)

IOW, I could see "ap_buckets.h" just as easily...

> When I commit this, I am VERY likely
> to also commit an apr_buf or ap_buf that just defines ap_bucket to be
> NULL.  This will be a placeholder file, because it is necessary.  Is that
> ok?

Makes sense, and I see your motivation, but see below...

> > > 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.
> 
> Again, my thinking was to bring the types under one name.  I can live with
> removing this stuff, but I would rather not.

I see your point. However, a placeholder type for a bucket isn't quite
"enough" as I'd rather see a signature that looks like:

API_EXPORT(ap_status_t) ap_pass_brigade(ap_filter_t *filter,
                                        ap_bucket_brigade *bucket)

Specifically: the ap_filter_t contains the request_rec that the filter was
inserted on. This is important if you chain together filters from multiple
(sub)requests. The change in filter->r is the signal for the partitioning of
filters between (sub)requests.

Roy seems to argue that ->r shouldn't be present anywhere in there. I think
that position is too idealistic, so I'm not listening to it :-)  No,
seriously, if we *can* get to a point where the request_rec isn't needed in
a filter, then it will be easier to remove the sucker from the structure
than from the signatures. [because (theoretically) nobody is referring to
->r any more]

But as I mentioned in the other note, we can simply tweak these typedefs and
signatures with our specific "filter system" patches.

>...
> > > API_EXPORT(int) ap_pass_brigade(request_rec *r, ap_filter_t *filter, 
> > >                                  ap_bucket_brigade *bucket);
> > 
> > Please remove this for now.
> 
> I'll change the files now to have those differences.  :-)

I'm not clear which differences you mean :-) ... but I think we're on track
for that ever elusive "It's Miller Time!"

I'll just wait for your update cuz it seems we're "aligned" now on this part
of the filtering. (woo!)

Cheers,
-g

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

Re: [PATCH] Filter registration.

Posted by rb...@covalent.net.
> > 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 ?

Yes

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

I know.  I included those in this patch, because we require some kind of
ap_bucket type.  Knowing that, and thinking that apr_buf or ap_buf is a
likely name, I decided to add this.  When I commit this, I am VERY likely
to also commit an apr_buf or ap_buf that just defines ap_bucket to be
NULL.  This will be a placeholder file, because it is necessary.  Is that
ok?

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

Again, my thinking was to bring the types under one name.  I can live with
removing this stuff, but I would rather not.

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

Same reasoning as above.

> 
> >...
> > 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").

Ok, that makes sense.

> 
> >...
> > API_EXPORT(int) ap_pass_brigade(request_rec *r, ap_filter_t *filter, 
> >                                  ap_bucket_brigade *bucket);
> 
> Please remove this for now.

I'll change the files now to have those differences.  :-)

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------




Re: [PATCH] Filter registration.

Posted by Greg Stein <gs...@lyra.org>.
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/