+1 (concept)
> On Oct 6, 2015, at 11:53 AM, Yann Ylavic <ylavic....@gmail.com> wrote:
>
> On Tue, Oct 6, 2015 at 5:34 PM, Graham Leggett <minf...@sharp.fm> wrote:
>>
>> apr_bucket_simple_copy() looks wrong - in theory we should have a proper
>> copy function that does the right thing with the second copy, for example by
>> not copying the pool. If we blindly copy the pool (or the request containing
>> the pool) I see nothing that would prevent an attempt to free the pool twice.
>
> Agreed, we probably need something like this:
>
> Index: server/eor_bucket.c
> ===================================================================
> --- server/eor_bucket.c (revision 1707064)
> +++ server/eor_bucket.c (working copy)
> @@ -91,6 +91,17 @@ static void eor_bucket_destroy(void *data)
> }
> }
>
> +static apr_status_t eor_bucket_copy(apr_bucket *a, apr_bucket **b)
> +{
> + *b = apr_bucket_alloc(sizeof(**b), a->list); /* XXX: check for failure?
> */
> + **b = *a;
> +
> + /* we don't wan't request to be destroyed twice */
> + (*b)->data = NULL;
> +
> + return APR_SUCCESS;
> +}
> +
> AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
> "EOR", 5, APR_BUCKET_METADATA,
> eor_bucket_destroy,
> @@ -97,6 +108,6 @@ AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_
> eor_bucket_read,
> apr_bucket_setaside_noop,
> apr_bucket_split_notimpl,
> - apr_bucket_simple_copy
> + eor_bucket_copy
> };
>
> --
>
> Regards,
> Yann.
