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* =)
Whatever...
[ 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
changes.
> > 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
