httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <cliffwool...@yahoo.com>
Subject Re: appending to the content brigade
Date Fri, 24 Aug 2001 18:43:28 GMT
On Fri, 24 Aug 2001, Eric Prud'hommeaux wrote:

> I'm implementing a content filter which wraps the content in multipart
> mime. I can generate a separator, my data, separator, original payload
> and separator. The problem is that if I just concatonate the brigades,
> I end up with an EOS before my final separator.

All you have to do is delete the EOS bucket and then do your concat.
That's a constant time operation as opposed to a foreach loop which is a
linear time operation.

> This works (the final bucket gets sent down the wire) but, if I
> understand the meaning of EOS I won't get into heaven this way.

Yeah, that's definitely bad news.

> I figured I could copy all but the EOS from the original payload to
> the new brigade:
>
> -    APR_BRIGADE_CONCAT(bsend, bPayload);
> +    APR_BRIGADE_FOREACH(e, bPayload) {
> +	if (!APR_BUCKET_IS_EOS(e))
> +	    APR_BRIGADE_INSERT_TAIL(bsend, e);
> +    }

This is broken because you're modifying the list pointers of e, which
APR_BRIGADE_FOREACH() doesn't support.  The foreach macro uses e->next to
determine what the next bucket in bPayload is at the end of each
iteration, so you have to treat e as essentially const.  What you'd want
to do is something more like this:

    while (!APR_BRIGADE_EMPTY(bPayload)) {
        e = APR_BRIGADE_FIRST(bPayload);
        if (!APR_BUCKET_IS_EOS(e)) {
            APR_BUCKET_REMOVE(e);
            APR_BRIGADE_INSERT_TAIL(bsend, e);
        }
    }

You have to do the remove before inserting into bsend so that the bPayload
ring remains consistent.

But like I said, this is a linear time operation, when you can do it in
constant time, like this:

    eos = APR_BRIGADE_LAST(bPayload);
    if (APR_BUCKET_IS_EOS(eos)) {
        APR_BUCKET_REMOVE(eos);
    }
    else {
        eos = apr_bucket_eos_create();
    }

    APR_BRIGADE_CONCAT(bsend, bPayload);
    /* apr_brigade_destroy(bPayload); */

    e = apr_bucket_pool_create(boundary, boundryLength, r->pool);
    APR_BRIGADE_INSERT_TAIL(bsend, e);
    APR_BRIGADE_INSERT_TAIL(bsend, eos);

Note that if the last bucket is not eos, depending on how your filter is
supposed to work that might be an indication that you need to stash the
brigade away until you DO get an eos, because the content you're wrapping
with your mime headers hasn't all arrived yet.  Alternatively, you can
send down what you have now but a lack of eos means you shouldn't tack on
your boundary stuff yet.

> I hadn't even gotten as far as destroying bPayload, which I expected
> to be a bad idea.

It's okay.  It's actually totally unnecessary, though (which is why it's
commented out above, because bPayload is empty at that point and the
brigade itself will get cleaned up when the pool goes away.

> The byterange filter uses some function calls to
> copy pieces of the brigade. Is this the sanctioned approach?

I assume you're talking about apr_brigade_partition() or
apr_brigade_split()... you definitely don't need partition here (which
splits the brigade at a given byte offset).  apr_brigade_split() is
useful when you want to send part of the data you have in a single brigade
and hang onto the rest, but I doubt you'll need that here either.
There are plenty of uses of apr_brigade_split() in the core filters that
you can look at as an example.

Hope this helps!  Let me know if you have more questions.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Mime
View raw message