Re: cvs commit: apr-util/include apr_buckets.h

[Cliff Woolley <cl...@yahoo.com>]

On 10 Aug 2001 rbb@apache.org wrote:

>   +/**
>   + * Remove all zero length buckets from the brigade.
>   + * @param b The bucket brigade
>   + */
>   +#define APR_BRIGADE_NORMALIZE(b)       \
>   +    do {  \
>   +        apr_bucket *e; \
>   +        e = APR_BRIGADE_FIRST(b); \
>   +        if (e->length == 0) { \
>   +            apr_bucket *d; \
>   +            d = APR_BUCKET_NEXT(e); \
>   +            apr_bucket_delete(e); \
>   +            e = d; \
>   +        } \
>   +        e = APR_BUCKET_NEXT(e); \
>   +    } while (e != APR_BRIGADE_SENTINEL(b))
>   +
>    /*
>     * General-purpose reference counting for the various bucket types.
>     *

What if you call this on a brigade that's already empty?  You'll probably
segfault.  You should move the while test up to the top of the loop, I
think.

--Cliff

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

Re: cvs commit: apr-util/include apr_buckets.h

[Ryan Bloom <rb...@covalent.net>]

On Friday 10 August 2001 11:56, Cliff Woolley wrote:
> On 10 Aug 2001 rbb@apache.org wrote:
> >   +/**
> >   + * Remove all zero length buckets from the brigade.
> >   + * @param b The bucket brigade
> >   + */
> >   +#define APR_BRIGADE_NORMALIZE(b)       \
> >   +    do {  \
> >   +        apr_bucket *e; \
> >   +        e = APR_BRIGADE_FIRST(b); \
> >   +        if (e->length == 0) { \
> >   +            apr_bucket *d; \
> >   +            d = APR_BUCKET_NEXT(e); \
> >   +            apr_bucket_delete(e); \
> >   +            e = d; \
> >   +        } \
> >   +        e = APR_BUCKET_NEXT(e); \
> >   +    } while (e != APR_BRIGADE_SENTINEL(b))
> >   +
> >    /*
> >     * General-purpose reference counting for the various bucket types.
> >     *
>
> What if you call this on a brigade that's already empty?  You'll probably
> segfault.  You should move the while test up to the top of the loop, I
> think.

It shouldn't seg-fault.  Looking at the logic, the BRIGADE_FIRST will always work,
regardless of how many buckets are in the brigade.  The length of the sentinel should
never be 0, and if it is, there is a bigger bug.  And, APR_BUCKET_NEXT should always
provide something, even if we are operating on the sentinel.

So, I don't think this is a problem.  Of course, feel free to make the change, the logic
is identitical.  It makes even less of a difference now that Doug added the exterior do {}
while loop.

Ryan


_____________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
-----------------------------------------------------------------------------