As I've mentioned before, I share the same concerns as Greg Marr with this particular approach (the transient buckets and large copies; although his first example sounds more like a bug (which is okay) than an inherent design issue).
Re: the followups. Again, I'm with Greg. We can't create a brigade API and then say it is Apache-internal, or that people shouldn't use it. That indicates a problem in the design/approach itself. Cheers, -g On Thu, Jan 25, 2001 at 05:28:13PM -0500, Greg Marr wrote: > At 04:51 PM 01/25/2001, [EMAIL PROTECTED] wrote: > > > FYI: this has the potential to fail miserably: > > > >I need to think about these, but I think you are correct. > > I can provide a walk-through of the code if you want. I decided it > was too long to post to the list. > > >I am not proposing that this is bullet-proof code, just that it is > >an idea that should be considered. > > I started out trying to figure out how you were successfully > memcpy'ing a block of unknown size into a fixed size buffer, then > stumbled into the check_ function. Those popped up while I was > reading through it. > > > > A third failure is that check_brigade_flush creates a transient > > > bucket and sticks it in the brigade. Here's where this can fail: > > > > > > char buf[APR_BUCKET_BUFF_SIZE * 2 + 1]; > > > > > > apr_vsnprintf(buf, APR_BUCKET_BUFF_SIZE * 2 + 1, fmt, va); > > > > > > return apr_brigade_puts(b, buf); > > > >This is not an issue, since it will never happen. Remember, that I > >said up front that these functions are not designed to be called by > >programmers. These are designed to be called by Apache's internal > >functions. In this case, the programmer has called either ap_f* or > >ap_r*, which passes things down the brigade. > > Well, they're in APR, and being used by Apache programmers, so I > don't see how they can be "designed to be called by Apache's internal > functions." > > >However, you are correct that in general this is an issue, the > >solution is to always copy the data, but that has other > >drawbacks. So, there are two answers to this that I see. #1, tell > >the programmer that (s)he is out of luck if (s)he doesn't understand > >the code. #2, actually copy all the data. > > > >I have no problem telling the programmer they are out of luck, this > >is open source after all. I dislike copying the data, because it > >means we could chew a lot of memory very quickly. > > So then the rule has to be that if call a apr_brigade_* function that > can generate more than APR_BUCKET_BUFF_SIZE data in a single call, > then that brigade has to be output before the buffer is reused or > goes out of scope? > It's hard to see how that fits well into a general brigade API. > > Would a better solution perhaps be that the apr_brigade_* functions > will always copy the data, and leave it up to the ap_f* functions to > provide the optimization of using the bucket API directly for large > blocks of data? That seems to be a much safer way of doing things. > > One other optimization that I noticed would probably be useful would > be to have apr_brigade_putstrs written at the same level as > apr_brigade_write such that you save the memcpy of apr_brigade_write > after the apr_vsnprintf. > > -- > Greg Marr > [EMAIL PROTECTED] > "We thought you were dead." > "I was, but I'm better now." - Sheridan, "The Summoning" -- Greg Stein, http://www.lyra.org/
