On 26 August 2016 at 14:27, Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com> wrote:
> Ivan Zhakov <ivan_at_visualsvn.com> writes:
>
>>> @@ -3266,15 +3264,13 @@ static svn_error_t * __attribute__((war
>>> close_filter(void *baton)
>>> {
>>> diff_ctx_t *dc = baton;
>>> - apr_bucket_brigade *bb;
>>> apr_bucket *bkt;
>>> apr_status_t status;
>>>
>>> /* done with the file. write an EOS bucket now. */
>>> - bb = apr_brigade_create(dc->pool, dc->output->c->bucket_alloc);
>>> bkt = apr_bucket_eos_create(dc->output->c->bucket_alloc);
>>> - APR_BRIGADE_INSERT_TAIL(bb, bkt);
>>> - if ((status = ap_pass_brigade(dc->output, bb)) != APR_SUCCESS)
>>> + APR_BRIGADE_INSERT_TAIL(dc->bb, bkt);
>>> + if ((status = ap_pass_brigade(dc->output, dc->bb)) != APR_SUCCESS)
>>> return svn_error_create(status, NULL, "Could not write EOS to filter");
>>>
>> As far I understand apr_brigade_cleanup() should be called after
>> ap_pass_brigade(). So code should be something like:
>> [[[
>> APR_BRIGADE_INSERT_TAIL(dc->bb, bkt);
>> status = ap_pass_brigade(dc->output, dc->bb);
>> apr_brigade_cleanup(dc->bb);
>> if (status != APR_SUCCESS)
>> return svn_error_create(status, NULL, "Could not write EOS to filter");
>> ]]]
>
> Thanks, I will add the missing call in trunk.
>
> While it's not strictly required here (this is the stream's close_fn(), and
> we always call apr_brigade_destroy() upon returning from the deliver()
> hook), this may not hold in the future. Or someone might improperly use
> this code as a reference.
>
Ok, thanks! I added my +1 for backport anyway, since there is no
actual problem/memory leak in this particular case.
--
Ivan Zhakov
Received on 2016-08-26 13:30:07 CEST