On 08/28/2019 04:20 PM, Joe Orton wrote:
> On Wed, Aug 28, 2019 at 02:24:40PM +0200, Ruediger Pluem wrote:
>> On 08/06/2019 05:41 PM, [email protected] wrote:
>>> Author: jorton
>>> Date: Tue Aug 6 15:41:22 2019
>>> New Revision: 1864526
> ...
>>> + ret = apr_brigade_length(ctx->bb, 1, &got);
>>> + if (ret || got > want) {
>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, ret, f->c,
>>> APLOGNO(10185)
>>> + "RemoteIPProxyProtocol header too long, "
>>> + "got %" APR_OFF_T_FMT " expected %"
>>> APR_OFF_T_FMT,
>>> + got, want);
>>> + f->c->aborted = 1;
>>
>> Shouldn't we do apr_brigade_destroy(ctx->bb) here as well like below?
>
> The apr_brigade_destroy() calls should be all redundant in the failure
> cases AFAICT. Can you see a reason why they need to be explicitly
> destroyed prior to the pool cleanups running?
>
Not really. I just noticed that we handle it differently. So I guess just
removing it in the second case would also
serve the purpose of doing the same thing in case we fail.
Regards
Rüdiger
