Re: Kernel bugs found using inspect tool

From: Peter T. Breuer (ptb@it.uc3m.es)
Date: Wed Feb 23 2000 - 09:24:12 EST


Trivia generate such discussions! It's the law of the meeting ...

"A month of sundays ago almesber@lrc.di.epfl.ch wrote:"
> Peter T. Breuer wrote:
> > It would be even nicer if someone wrote
> > aal5 = (vcc->qos.aal == ATM_AAL5);
> > if (!aal5) { ...
>
> Taking the assignment out of the if sounds like a good idea. Adding
> parentheses much less so. If people find C's precendence rules difficult

I added them only after looking at what it looked like without them.
But you are right, once out of the if condition, it's plain that it
must be an assignment, parens or not.

But there are plenty of places in the kernel code where people write
if ((a == b) || (c == d) || (e < f)) ... It drives me batty, but Linus
does not seem to mind.

> > or even used a switch, since it's that kind of thing here.
>
> Not quite - the hardware doesn't know anything but AAL5 or non-AAL5.

That's alright. A switch is fine for the if/then/else even in the case
of just one possibility and the default. It's a common style when the
test is against an integer constant.

> > Maybe it's just me, but this routine has if(aal5) all the way through,
> > and should probably be split into two.
>
> Then you'd also duplicate a lot of other, common, and quite hairy code.

Which should then be put into functional subunits?

> The driver is complex enough already ...

I already spent a couple of years (back a couple of years) retrofitting
atm codes into 2.0.*! I know.

Peter

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Wed Feb 23 2000 - 21:00:33 EST