httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bp...@pacbell.net>
Subject Re: cvs commit: httpd-2.0/modules/filters mod_include.c
Date Wed, 19 Jun 2002 15:43:16 GMT
On Wed, 2002-06-19 at 08:07, Cliff Woolley wrote:
> On 19 Jun 2002 brianp@apache.org wrote:
> 
> >                ap_save_brigade(f, &ctx->ssi_tag_brigade, &tag_and_after,
r->pool);
> >   +            if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(ctx->ssi_tag_brigade)))
{
> >   +                apr_bucket *new_eos;
> >   +                /* Make sure there's no EOS at the end of the set-aside
> >   +                 * brigade, because we may later prepend it to some
> >   +                 * other brigade
> >   +                 */
> >   +                APR_BUCKET_REMOVE(APR_BRIGADE_LAST(ctx->ssi_tag_brigade));
> >   +
> >   +                /* And put an EOS on the brigade that we're about to pass
> >   +                 * to the next filter.
> >   +                 */
> >   +                new_eos = apr_bucket_eos_create((*bb)->bucket_alloc);
> >   +                APR_BRIGADE_INSERT_TAIL(*bb, new_eos);
> >   +            }
> 
> I'd have to look more closely, but I think this leaks the EOS bucket that
> you removed from ctx->ssi_tag_brigade.  To fix the leak you'd use
> apr_bucket_delete() instead of APR_BUCKET_REMOVE().  In either case, these
> are macros and it's not safe to put the APR_BRIGADE_LAST() inside the
> APR_BUCKET_REMOVE( ... ).  Make it two separate lines.  But finally, why
> destroy the EOS bucket you already have?  Just remove it from the first
> brigade and reinsert into the second brigade.

I was concerned about whether the EOS from the first brigade would have
a long enough lifetime for the new brigade.  But that's not actually a
problem, so I can definitely just move the EOS from the old brigade to
the new one.

As Justin pointed out on IRC, though, most of the logic to setaside
the brigade is probably unnecessary because there's currently no way
for that same filter instance to ever get called again with a different
brigade, so the incomplete tag won't ever be completed.

I'll patch up the code after work today.

--Brian



Mime
View raw message