httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Manoj Kasichainula <ma...@io.com>
Subject Re: [PATCH] ap_add_filter
Date Sun, 20 Aug 2000 12:50:54 GMT
On Sat, Aug 19, 2000 at 07:50:34AM -0700, rbb@covalent.net wrote:
> Huh?  Even in your design you have two separate calls to the same
> filter.  having me->filter_child_head and me->filter_chain_tail point to
> the same function point is the exact same as inserting the same filter
> twice.

No it's not. if head == tail, then you have a one element list.
filter_child_head->next then points outside the "personal" chain and
back to the main chain.

> > How does current_filter become null under the scheme I'm proposing?
> 
> At some point, there are no more filters in the sub_list.  At that point,
> current_filter becomes NULL.

No. See my earlier diagram. This is similar to Greg's subrequest
design; filter_chain_tail->next points back to the main chain.

> If I understand what you are proposing, graphically, it looks like the
> following:

All of the diagrams you drew were wrong. :) I think I may understand
where the confusion is. I drew a dataflow diagram earlier, which may
have been construed to be the actual filter diagram. Also, I'm digging
more through the code now, so hopefully my terminology will be better.
I'm also going to be painfully explicit here.

Assuming PHP is the filter with the subchain in this case:

As far as everything but PHP itself (or data traversing through it) is
concerned, the chain looks like:

php -> core

This is an important point. Simple filters don't create subchains, and
when complex filters do, the rest of the world doesn't have a clue
about it.

Now, when the filter chain is constructed initially, before PHP
decides it wants more filters, dig a little deeper and you see the
data flow chain:

  _________
 |php    __|___next___>core
 | !    /  |
 | V   /   |
 | chain   |
  ---------

Now the "!" dashed line is *not* a next pointer. Data flows in that
direction, but that's because of php's filter function

php_filter->filter_func(ap_filter_t *f, ap_bucket_brigade_t *bb) {
    ap_pass_brigade(f->ctx->filter_chain_head, bb);
}

Now, in an initialization function (which would be the filter
registration function if ap_add_filter returned the ap_filter_t it
created), we'd say:

f->ctx->filter_chain_head = f->ctx->filter_chain_tail = php_impl;

so that "chain" above is just the real PHP implementation.

Now, this does have a couple of implications. One is that the code
either needs a way to create ap_filter_t's without putting them into
the main filter chain, or a way to ap_add_filter to an arbitrary
chain. I prefer the former.

The other implication is that the main php filter is really a shell
filter, with the real goop in the php_impl filter. From the code I've
perused so far, this looks like a difference of maybe 5-10 lines of code.

Now, look at the filter's personal chain: After the php_impl
assignment above, you get:

           php_impl  -------next------>core
             ^   ^
   _________/     \___
  /                   \
filter_chain_head      filter_chain_tail

filter_chain{head,tail} both point to the php_impl filter. The purpose
of having both a head and tail pointer is to allow the
insert-at-beginning and insert-at-end operations to be really easy,
and to allow searches through the list to have an endpoint, because
filter_chain_tail->next == core, *not* NULL.

So, if php decides to add to the end of its own filter chain, it uses
a standard linked list function to add to the end of the list. I
listed that in the original message. Here it is again, assuming my
memory is good:

{
new_f->next = filter_chain_tail->next;
filter_chain_tail->next = new_f;
filter_chain_tail = new_f;
}

So the chain looks like

   php_impl ----next----> new_f ----next---------> core
        ^                  ^
   ____/                    \___
  /                             \
filter_chain_head              filter_chain_tail

And so on. Note that the next pointers always work, so ap_pass_brigade
always works. But, php->next still == core: the main filter chain
isn't touched, though php->next isn't actually used because of how
php's filter_func is written.

So, to summarize, this design suggests a few changes:

- Instead of ap_add_filter, just have an ap_make_filter, and let
  whatever made the filter add the filter to its own chain using some
  standard linked list functions like add-after-this-thing or
  add-to-tail.

- Any filters that want to affect the data flow put a
  filter_chain_{head,tail} in the filter context.

- These filters also use an extra ap_filter_t. I don't think this
  requires any significant extra code. You include the 1-line proxy
  filter_func above, create a filter based on it, and you register
  this filter_func in ap_register_filter instead. I think that's
  it.

This is not much extra code, and I think it's actually less
complicated, because the explanation of how the main
filter chain is setup is a seperate one from how filters run their
data through other filters.

The benefits:

- The argument between you and Greg goes away, because a filter can be
  added anywhere you want. There's more freedom because the filter has
  it's "zone" of the dataflow staked out to do whatever it wants.

- Filters no longer have to muck around with the main filter chain.  I
  think this is just a cleaner design. Filters only look at their own
  world, and the boundaries of what they can and can't do are clearly
  marked. Having lots of entities touching a single data structure
  when it can easily be avoided just seems wrong to me.

- We've effectively taken the whole problem of how to allow filters to
  add other filters out of Apache completely, and put it into the
  filter, with very little added code (and we can make that 10 lines
  of code shared if we want). A filter can use a completely different
  mechanism (and I even considered another one which is even cleaner
  than this which allows the main filter chain to be completely
  mangled at-will by Apache without problems, but I'll bring that one
  up later :)

- This scheme handles the subrequest EOS problem from the STATUS file
  as well.

I was going to add an example of how to do this to one of the existing
filters; but that will need to come later.


Mime
View raw message