> 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"
