apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@covalent.net>
Subject Re: [PATCH] bucket type "capabilities" flags
Date Wed, 20 Jun 2001 03:41:07 GMT

> > At this point, you are trying to second guess the bucket type.  If the
> > bucket doesn't implement setaside, then it doesn't need setaside.
>
> That's not true... it it doesn't need setaside, it will use
> apr_bucket_setaside_noop() [which counts as an implementation for flag
> purposes]; apr_bucket_setaside_notimpl() means the bucket type either
> needs it and it's not done yet, or it cannot be supported by the bucket
> type.

But we already stated that not having a function isn't a good reason for
the flag.  If it's needed and not done yet, then the bucket shouldn't be
in released code, and a filter can't take advantage of it.

> > Those aren't benign.  Content_length has every right to call read, or
> > setaside on a bucket.  So does byterange.
>
> They have the *right* to, but that doesn't mean the necessarily *will*.

Yeah, and you could determine if it will or not, but that is adding a LOT
of logic that can change quickly to the handler.  We haven't exactly had a
stellar history determining if the content length filter should setaside
data or not.

> > Yes, I see how it works, but I don't believe this is the correct solution.
> > How did you open the file originally?  Did you use apr_file_open, or did
> > you use native calls?  If you used apr_file_open, then APR already has
> > enough information to do exactly what you are doing without the custom
> > bucket type.  My concern, is that you are building logic for when a file
> > can be read and when it can't into a custom bucket, but that logic is
> > actually very important for APR to have.  This does make the file_bucket
> > slightly more complex, because now that bucket type has to handle a new
> > error from apr_file_read(), but it should reduce the amount of duplicate
> > code, and it removes the requirements for the flags.
>
> Okay, I'm willing to try to implement it as an extension to the regular
> file buckets.  apr_file_open() already has an APR_XTHREAD (or something
> like that) flag, though only Win32 currently does anything with it.  At
> any rate, getting this to work will require adding a pool parameter to
> apr_bucket_file_create() as I suggested a while back; that is the pool
> that should be used by file_read() to MMAP the file or to re-open it for
> single-threaded reading.  I can get the filename from the apr_file_t (I
> think), but I can't get that extra pool.  Note that it will generally be
> r->pool for Apache purposes... but without passing it in as a parameter to
> the buckets code, file_read() will have no idea which pool to use.

Add the pool to the apr_bucket_file_create call.  I have no problem doing
that.

Ryan

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


Mime
View raw message