On Thu, Aug 12, 2004 at 02:27:03PM -0700, Justin Erenkrantz wrote:
> --On Thursday, August 12, 2004 5:03 PM -0400 Glenn Strauss
> <[EMAIL PROTECTED]> wrote:
>
> > BRIGADE_NORMALIZE skips the bucket after a 0-length bucket.
> >In doing so, it might skip a 0-length bucket following it. If the last
>
> IMHO, the cleanest correct fix is to just add an else clause. At the cost
> of more variables, you could always use d and forgo the else clause, but I
> think that'll just confuse readers of the macro even more. We also can't
> accept C99 extensions. *shrug*
>
> We probably never hit this odd corner case in practice though because
> core_input_filter only has one of the following: one empty bucket, one
> socket bucket, one heap bucket, or a heap and a socket bucket: two empty
> buckets won't happen due to the design. FWIW, BRIGADE_NORMALIZE was thrown
> out of apr-util because it was really bad to begin with: hence the comment
> ("This is bad.") and the localization of it to only server/core.c. It was
> never meant to be an example - yet, this is a minor bug more than anything
> serious. -- justin
Right. I didn't say it was a problem in practice.
I did say that it was a terrible piece of code, and since this list
often refers people to "look at the code", it should be fixed, IMHO.
It is a _bad_ and _broken_ example of how to loop through a brigade.
As for C99 extensions, I understand that it is not available on all
platforms, but why can't new code checked in include the 'restrict'
keyword? Just like there is an APR_INLINE macro, why isn't there
an APR_RESTRICT macro indirection? Would a patch implementing such
in APR be accepted for APR 1.0?
Cheers,
Glenn
