> On Wednesday 02 January 2002 10:34 am, Bill Stoddard wrote:
> > I see this pattern several places in the code:
> >
> > rc = apr_get_brigade();
> > if (rc != APR_SUCCESS) {
> > fail;
> > }
> > /* Is the brigade empty? */
> > if (APR_BRIGADE_EMPTY(b)) {
> > fail;
> > }
> > /* Get first bucket */
> > e = APR_BRIGADE_FIRST(b);
> >
> > /* Is the bucket an EOS? */
> > if (APR_BUCKET_IS_EOS(e)) {
> > }
> >
> > /* Does the bucket have zero length? */
> > if (e->length == 0) {
> > }
> >
> > You have to make these checks each and everytime we fetch a brigade which seems
> > unintuitive. Should a apr_bucket_read() call on a bucket with e->length == 0 cause
>a
seg
> > fault? Seems this check is masking a deeper problem.
>
> A bucket_read on a bucket with e->length == 0 should not seg fault, but that isn't
> what is happening here. What is actually happening is that we are calling bucket
> read on the sentinel which is seg faulting. The problem is that the sentinel is kind
> of a fake bucket, so it doesn't actually have a bucket->type field, but we are
> trying to use the bucket->type to find the read function, which causes us
> to segfault.
>
> There are a few possible solutions:
>
> 1) Fix the apr_bucket_read macro to check if this bucket is the sentinel,
> and fail appropriately. If we do this, we will add one if to every bucket_read
> operation.
>
> 2) Make the sentinel a more complete bucket along with a bucket->type field.
> This bucket->type will include a no-op for the read function.
>
> I personally prefer option 2, but it may be a bit harder to implement. I still
>haven't
> wrapped my head around the ring macros yet.
>
> Ryan
Yea, 2) looks good in concept, but I'm unsure how APR_BUCKET_REMOVE would be
implemented
to special case removing a sentinel bucket. The latest fix checked into proxy_util
should
avoid the segfault in ap_proxy_string_read() (Thanks Adam!)
I am beginning to collect some notes to put into a 'how-to' document for developers.
There
are certain patterns that repeat all over the server and it would be good to codify
that
knowledge in one reference document.
Bill
