httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <>
Subject Re: [PATCH] some fixes for ap_http_filter
Date Fri, 05 Oct 2001 02:50:33 GMT
On Thu, Oct 04, 2001 at 07:13:16PM -0700, Aaron Bannert wrote:
> Huh? That's a bit of a stretch. If this is going to turn into a style
> war, let it be spoken from the Apache Style Guide:
> "Opening braces are given on the same lines as statements, or on the
> following line at the start of a function definition."

I'd like to point out the other bazillion places where this happens.
I don't care.  I'm just pointing out that if you want to change
the style, change style without changing anything else.  That
seems to be the operative rule that most people work under.

Furthermore, I won't commit any patches that change the braces, so 
there!  =)  When you finally get commit access or find someone who 
agrees to enforce this point (I'm sure Ryan will do so), I don't 
care.  It isn't a big enough of a deal.  *I DON'T CARE*  =)  

[ Please realize that I'm writing this laughing... ]

> > As an aside, I'd rather not see any style changes be in the same
> > commit/patch as something that changes bytecode - unless enough
> > of the function is changed to warrant the rewrite.  I've been
> > told many times not to change style in a patch - and for the most 
> > part, I try not to do that anymore.  I'd just like to reinforce 
> > that behavior in others.  =)
> My patch changes no visible functionality. Part of the patch simply added
> comments, almost *ALL* of it was to improve robustness and readability.
> Minor style changes definately fit into this.

I thought the comments were good.  I just didn't like the style 

> > No, I think that if you are not familiar that the C construct 
> > (ctx->remaining) is equivalent to (ctx->remaining != 0) than I
> > don't really have any sympathy for you.  IMHO, we're not here to 
> > teach people C.  =)
> Bzzzt, wrong! Those are not semantically equivalent statments. The !
> operator treats the operand as a boolean, the == operator treats the
> operands as the same type. Let me do some english translations:
> if (!ctx->remaining) {
>  "If ctx->remaining is not true..."
> if (ctx->remaining <= 0) {
>  "If ctx->remaining is less than or equal to zero..."
> There is a huge semantic difference there, and it plays directly into the
> readability of the code.

Yoohoo, look again - that's not what I said.  =)  I'm not disagreeing 
that <= 0 is different (of course it is).  I'm saying that != 0 is 
identical to not having the explicit comparison (and I bet there is
someone out there that will prove me wrong).  Also, there is no 
boolean type in C.  =)  You *are* thinking of Java - I knew it...

I'm also saying that given the context < 0 is invalid and we should
fix it (as described below).

> > So, I think we should fix that problem (so that we can't read 
> > past the end of the body) - not hide it by checking for negative 
> > values.  So, I think we should check ctx->remaining < readbytes
> > before we call ap_get_brigade in ap_http_filter.  If ctx->remaining
> > is less than readbytes, only read in ctx->remaining.  How does
> > that sound?  -- justin
> That would also be satisfactory. As long as we aren't making implicit
> assumptions on our input.

Right.  I think this is the way to go.  It makes sense and enforces
ctx->remaining to never be < 0 no matter what higher-level filters
request.  =)

At least we can agree on something.  =)  Mark this date down in 
history.  -- justin

View raw message