On Mon, Apr 25, 2005 at 06:12:04PM -0500, Rici Lake wrote:
> I guess that depends on whether most filters use and recycle pass
> brigades. I suspect that most internal filters don't (and possibly have
> the model which Joe Schaefer describes, where the brigade is simply
> passed through), whereas most third-party filters probably either do or
> should. Having now looked at quite a few output filters, I've come to
> the conclusion that the normal model is to create and destroy brigades
> on every invocation, which I can't help but think is a lot less
> efficient than it could be.
>
> But I actually disagree with the premise: the issue is not how
> prevalent the case is, it is how likely it is to create a bug which is
> triggered by an interaction between unrelated modules. Mitigating the
> creation of such bugs is, in my mind, an important part of component
> API design; particularly if the mitigation is low-cost. Of course,
> everyone has their own concept of beauty, and mine is probably the
> least important of anyone participating in this debate.
This is really just a "should we provide extra API guarantees to prevent
users shooting themselves in the foot" debate, right? Let's agree to
disagree :)
> In any event, a draft docstring. This does not reflect my view, and I
> don't know if there is actually a consensus view, but it may reflect
> the current state of affairs:
It got linewrapped (format=flowed?) but otherwise I think this looks
good - thanks a lot. Final chance for any dissenter to object? Else
I'll commit this...
(attached un-line-wrapped version)
Index: util_filter.h
===================================================================
--- util_filter.h (revision 158730)
+++ util_filter.h (working copy)
@@ -103,8 +103,8 @@
* @name Filter callbacks
*
* This function type is used for filter callbacks. It will be passed a
- * pointer to "this" filter, and a "bucket" containing the content to be
- * filtered.
+ * pointer to "this" filter, and a "bucket brigade" containing the content
+ * to be filtered.
*
* In filter->ctx, the callback will find its context. This context is
* provided here, so that a filter may be installed multiple times, each
@@ -120,10 +120,15 @@
* or output filter chains and before any data is generated to allow the
* filter to prepare for processing.
*
- * The *bucket structure (and all those referenced by ->next and ->prev)
- * should be considered "const". The filter is allowed to modify the
- * next/prev to insert/remove/replace elements in the bucket list, but
- * the types and values of the individual buckets should not be altered.
+ * The bucket brigade always belongs to the caller, but the filter
+ * is free to use the buckets within it as it sees fit. Normally,
+ * the brigade will be returned empty. Buckets *may not* be retained
+ * between successive calls to the filter unless they have been
+ * "set aside" with a call apr_bucket_setaside. Typically this will
+ * be done with ap_save_brigade(). Buckets removed from the brigade
+ * become the responsibility of the filter, which must arrange for
+ * them to be deleted, either by doing so directly or by inserting
+ * them in a brigade which will subsequently be destroyed.
*
* For the input and output filters, the return value of a filter should be
* an APR status value. For the init function, the return value should
@@ -299,9 +304,13 @@
* Pass the current bucket brigade down to the next filter on the filter
* stack. The filter returns an apr_status_t value. If the bottom-most
* filter doesn't write to the network, then ::AP_NOBODY_WROTE is returned.
- * The caller relinquishes ownership of the brigade.
* @param filter The next filter in the chain
* @param bucket The current bucket brigade
+ *
+ * @remark Ownership of the brigade is retained by the caller. On return,
+ * the contents of the brigade are UNDEFINED, and the caller must
+ * either call apr_brigade_cleanup or apr_brigade_destroy on
+ * the brigade.
*/
AP_DECLARE(apr_status_t) ap_pass_brigade(ap_filter_t *filter,
apr_bucket_brigade *bucket);
