On Thu, Aug 12, 2004 at 10:20:14AM -0700, Justin Erenkrantz wrote: > --On Thursday, August 12, 2004 2:51 AM -0400 Glenn Strauss > <[EMAIL PROTECTED]> wrote:
> >of code duplication between modules. For example, the behavior of > >line-mode is vauge and requires that callers re-parse the brigade > >to check for complete lines. I've got a whole litany of things, > > It's for the reason listed below... > > >including bad code examples of brigade parsing in the Apache core, > >but that's another post. > > Well, pointers as to where things aren't right are certainly appreciated. Certain design decisions result in situations that are annoying and difficult to work with. (Some moreso than others, and I am in no way suggesting that all of these can easily be fixed or even that they should be fixed) - The way that all line-mode reads work, the caller must re-parse the returned brigade, repeating exactly what upstream filters did. The caller must read every bucket, check the length, and check that it is LF terminated, when the upstream filters already knew those answers. And because partial lines are returned, the caller must do this parsing even when upstream knows that the line is not complete. Also, because partial lines are returned, upstream filters do not know when a line length limit has been reached if portions of the line had been returned previously. - ap_http_filter() blocks on read chunk lines and chunk trailers, even when APR_NONBLOCK_READ is requested. Actually, anything that uses ap_rgetline() or ap_get_mime_headers() will block. - apr_brigade_split_line() might return more than readbytes The entire bucket is returned when readbytes is exceeded, instead of bucket splitting at the limit. - apr_brigade_split_line() in core_input_filter() uses HUGE_STRING_LEN as maximum line length; regardless of what caller specified in readbytes. - Every filter that buffers buckets must reinvent the wheel to protect against the buildup of many, many buckets each containing 1-byte, or other bucket fragmentation fun. - AP_MODE_SPECULATIVE only seems to work for reading bytes from connection filters. If it were used during chunking (it is not in practice), then it might return portions of chunk lines or chunk trailers along with the data, and downstream caller would not know the difference. In other words, AP_MODE_SPECULATIVE is not useful to too many filters unless the filter knows certain things about where it is in the filter stack. That means that ap_rgetline() can not be used except by those same filters that are hyper-aware of their location in the filter stack and ramifications of pulling data speculatively. As it is, AP_MODE_SPECULATIVE is a noticeable amount of extra work for the SSL and HTTP_IN filters. - apr_bucket_read() does not bother to look at the value in &len that was passed in. It always reads APR_BUCKET_BUFF_SIZE (8000 bytes) in the case of sockets and pipes. That's good for efficiency, but there is no way for a caller to specify "only read X bytes" when it really, really means "only read X bytes" - the close() routines for pipes and sockets burn a system call if the fd is already closed and marked as such. This could easily be avoided by checking if the fd < 0 in the pipe or socket bucket before attempting the close() on the pipe or socket bucket fd That's all I can think of at the moment. Cheers, Glenn
