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] Take 2 of the http filter rewrite
Date Mon, 24 Sep 2001 20:42:27 GMT
On Mon, Sep 24, 2001 at 12:27:21PM -0700, Justin Erenkrantz wrote:
> On Mon, Sep 24, 2001 at 03:14:22PM -0400, Cliff Woolley wrote:
> > On Mon, 24 Sep 2001, Justin Erenkrantz wrote:
> > 
> > > The brigades should not be returning more data than is asked for
> > > ever.
> > 
> > Right.  But the only increment the API allows to ask for are one whole
> > bucket's worth at a time.

[ cliff referring to buckets and brigades; the real problem with the filters
  not the brigade API ]

> util_filter's ap_get_brigade takes in the length to read.  The

Unfortunately, it takes it as a pointer-to-length. That is wrong -- it
should just be an apr_off_t length value.

I changed that at one point in April or so. However, that broke something
else and Ryan reverted most of the patch. I forget the full reason there,
but it was definitely related to some of the other issues we've been
discussing around your patch over the past couple days.

> translation from httpd->apr_util in the core filter loses this 
> distinction.  The core filter should be splitting the buckets and 
> only returning the requested amount to HTTP_IN (and it'd have to 
> handle the 0 case when we want to read a line).

Absolutely. And to support the 0 case, I suggested a set of brigade
functions to help out:

  apr_brigade_getline(bb, line, sizeof(line))
  apr_brigade_pgetline(bb, &line, pool)
  apr_brigade_splitline(bb, &newbb)

The first two would use the last function to implement the getline process.
The reason for the first two forms is for memory management issues. You may
not want to copy the line into a pool, but on the other hand, you may want
the whole line even if it doesn't fit your buffer.

Note on the getline() case: you'd need to distinguish between "the line
filled the buffer" and "the line is longer than the provided buffer". Maybe
a returned length could indicate that somehow.

This getline stuff is in the httpd STATUS file under the item for revamping
the input filter system.

By making brigade functions, you could do a getline anywhere in the code.
Consider the case where you have a decompression filter and a higher filter
asks for a line. The filter would decompress into a buffer or a brigade,
then see the "0" and need to return a line. Having the function available
makes it easy for him. If you isolate the line-from-brigade functionality to
CORE_IN, then it will limit what others can do.

> Since the
> core is inherently connection-based, I see no reason why the
> buffering can't be done there.  Currently, it passes the
> socket bucket up the chain.

It certainly can. Zero complaints here. Hell... loud applause if you are
going to spend the time to fix it!

>...
> The problem is that HTTP_IN has the socket bucket to begin with (which 
> has -1 length - i.e. indeterminate).  In order to properly handle a 
> request-centric HTTP filter, that filter must not get the -1 bucket.
> It must be hidden by the core filter.

Yup.

Note that (strictly speaking) you may not even need the socket bucket. You
could read straight from the socket, create a bucket around that, and insert
the result into the passed-in brigade.

If you use a socket bucket, then you're going to have to have an internal
brigade. Then you're going to read, split, and shift some buckets from your
internal brigade over to the passed-in brigade.

YMMV, but consider the case of avoiding a socket bucket.

> (This is the way I see it...)  -- justin

Cool. Me too :-)

Like I said, I was just hoping that your patch would not require all of this
extra work. That it could be applied as an incremental improvement. But,
alas... that doesn't seem to be the case. So the first step would be
adjusting the return amounts, then applying your patch.

Cheers,
-g

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

Mime
View raw message