> > 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 [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 -------------------------------------------------------------------------------
