Kernel bugs found using inspect tool
Rupp, Ed J
the AT&T toolchest called 'inspect'. I found 10 near-certain
bugs which may or may not be causing any observable problems
and 13 probable bugs which I couldn't immediately tell if they
were just odd coding style or real errors. Most of the bugs
are in the drivers area which I guess is no suprise since these
drivers are less heavily travelled than the rest of the kernel.
I hope this is the right list to send this to.
===== BUGS ====
drivers/atm/nicstar.c: extra semicolon near line 505
if (pci_write_config_byte(pcidev, PCI_LATENCY_TIMER,
NS_PCI_LATENCY) != 0);
drivers/cdrom/aztcd.c: extra semicolon near line 1730
for (count = 0; count < AZT_TIMEOUT; count++);
drivers/char/joystick/joy-creative.c: extra semicolon near line 69
for (i = 0; i < 2; i++); {
drivers/net/acenic.c: extra semicolon near line 1090
if (!(ap->trace_buf = kmalloc(ACE_TRACE_SIZE, GFP_KERNEL)));
drivers/net/pcmcia/netwave_cs.c: bitwise-or in conditional context near line
1319
if ((dev == NULL) | (!dev->start))
drivers/net/tokenring/smctr.c: extra semicolon near line 2938
for(i = 0; ((i < 6) && (dev->dev_addr[i] == 0)); i++);
drivers/scsi/53c7,8xx.c: extra semicolon near line 3152
for (bp = (struct NCR53c7x0_break *) host->breakpoints;
drivers/sound/vidc_audio.c: extra semicolon near line 82
for (new2size = 128; new2size < newsize; new2size <<= 1);
drivers/telephony/ixj.c: extra semicolon near line 1976
if (ixj[board].play_mode != -1 && ixj[board].rec_mode != -1);
net/khttpd/security.c: extra semicolon near line 134
if (filp!=NULL);
===== Questionable ====
arch/ppc/math-emu/op-4.h: comparison takes precedence over assignment near
line 215
(x3 += ((x2 += ((x1 += ((x0 += i) < x0)) < x1) < x2)))
drivers/atm/eni.c: comparison takes precedence over assignment near line
1041
if (!(aal5 = vcc->qos.aal == ATM_AAL5))
drivers/char/sx.c: missing logical operator near line 627
for (i=0; i < TIMEOUT_1 > 0;i++)
drivers/char/sx.c: missing logical operator near line 633
for (i=0; i < TIMEOUT_2 > 0;i++) {
drivers/char/sx.c: missing logical operator near line 655
for (i=0; i < TIMEOUT_1 > 0;i++)
drivers/char/sx.c: missing logical operator near line 661
for (i=0; i < TIMEOUT_2 > 0;i++) {
drivers/net/fc/iph5526.c: missing logical operator near line 3773
for (i = 0; i < clone_list[i].vendor_id != 0; i++)
drivers/scsi/FlashPoint.c: bitwise-or in conditional context near line 4713
while ((hp_int = RDW_HARPOON((ioport+hp_intstat)) & default_intena) |
drivers/scsi/advansys.c: missing logical operator near line 7684
qdonep->remain_bytes <= scp->request_bufflen != 0) {
drivers/sound/lowlevel/aci.c: comparison takes precedence over assignment
near line 390
return (*(int *) arg = (status & 0x20) == 0);
drivers/sound/sscape.c: comparison takes precedence over assignment near
line 603
if (hw_config->irq > 15 || (regs[4] = irq_bits == 0xff))
drivers/video/atafb.c: comparison takes precedence over bitwise-and near
line 1198
if (par->HDB & 0x200 && par->HDB & ~0x200 - par->HDE <= 5) {
net/decnet/af_decnet.c: comparison takes precedence over assignment near
line 1842
if ((err = sock_error(sk) != 0))
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
Ricky Beam
>the AT&T toolchest called 'inspect'. I found 10 near-certain
>bugs which may or may not be causing any observable problems
>and 13 probable bugs which I couldn't immediately tell if they
Oh, this is funny!
(yes, I reorderd them)
...
>drivers/scsi/53c7,8xx.c: extra semicolon near line 3152
> for (bp = (struct NCR53c7x0_break *) host->breakpoints;
Full context:
for (bp = (struct NCR53c7x0_break *) host->breakpoints;
bp; bp = (struct NCR53c7x0_break *) bp->next); {
>drivers/char/joystick/joy-creative.c: extra semicolon near line 69
> for (i = 0; i < 2; i++); {
*grin* Those two are just classic! It's been 6 years since I've seen such
beautiful snafu's. (I used to teach FORTRAN labs.)
...
>drivers/net/pcmcia/netwave_cs.c: bitwise-or in conditional context near line
>1319
> if ((dev == NULL) | (!dev->start))
Oh well. This line has to be rewritten anyway. (dev->start no longer exists.)
>===== Questionable ====
>arch/ppc/math-emu/op-4.h: comparison takes precedence over assignment near
>line 215
> (x3 += ((x2 += ((x1 += ((x0 += i) < x0)) < x1) < x2)))
Eww... I'm too tired to read lines like that.
...
>drivers/char/sx.c: missing logical operator near line 627
> for (i=0; i < TIMEOUT_1 > 0;i++)
>drivers/char/sx.c: missing logical operator near line 633
> for (i=0; i < TIMEOUT_2 > 0;i++) {
>drivers/char/sx.c: missing logical operator near line 655
> for (i=0; i < TIMEOUT_1 > 0;i++)
>drivers/char/sx.c: missing logical operator near line 661
> for (i=0; i < TIMEOUT_2 > 0;i++) {
TIMEOUT_1 and TIMEOUT_2 are numbers, btw. This one wins a cardboard cookie.
(You'll be required to show up in person to collect it.)
...
>drivers/net/fc/iph5526.c: missing logical operator near line 3773
> for (i = 0; i < clone_list[i].vendor_id != 0; i++)
This one is the runner-up for the cardboard cookie.
...
>drivers/sound/lowlevel/aci.c: comparison takes precedence over assignment
>near line 390
> return (*(int *) arg = (status & 0x20) == 0);
But it didn't complain about (a few lines above):
aci_solo = !!*(int *) arg;
--Ricky
PS: Now if it could find stuff that isn't so "cosmetic"...
alme...@lrc.di.epfl.ch
> drivers/atm/nicstar.c: extra semicolon near line 505
> if (pci_write_config_byte(pcidev, PCI_LATENCY_TIMER,
> NS_PCI_LATENCY) != 0);
This one's for real and has been fixed a few kernels ago.
> drivers/atm/eni.c: comparison takes precedence over assignment near line
> 1041
> if (!(aal5 = vcc->qos.aal == ATM_AAL5))
No problem here.
- Werner
--
_________________________________________________________________________
/ Werner Almesberger, ICA, EPFL, CH werner.al...@ica.epfl.ch /
/_IN_N_032__Tel_+41_21_693_6621__Fax_+41_21_693_6610_____________________/
H. Peter Anvin
By author: Ricky Beam <jfb...@bluetopia.net>
In newsgroup: linux.dev.kernel
>
> But it didn't complain about (a few lines above):
> aci_solo = !!*(int *) arg;
>
What's wrong with this one? !! in C is the standard way to
"booleanize" a value; "!!foo" means the same as "foo ? 1 : 0" except
for precedence, and obviously the former is much briefer.
The line thus means the same thing as:
aci_solo = *(int *)arg ? 1 : 0;
-hpa
--
<h...@transmeta.com> at work, <h...@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Tim Waugh
> > drivers/atm/eni.c: comparison takes precedence over assignment near line
> > 1041
> > if (!(aal5 = vcc->qos.aal == ATM_AAL5))
>
> No problem here.
If aal5 really is supposed to be a truth value (as the use of ! suggests),
it would be really nice if someone would put in some parentheses.
Tim.
*/
Jes Sorensen
Ed> I ran the 2.3.38 distribution through a program from the AT&T
Ed> toolchest called 'inspect'. I found 10 near-certain bugs which
Ed> may or may not be causing any observable problems and 13 probable
Ed> bugs which I couldn't immediately tell if they were just odd
Ed> coding style or real errors. Most of the bugs are in the drivers
Ed> area which I guess is no suprise since these drivers are less
Ed> heavily travelled than the rest of the kernel. I hope this is the
Ed> right list to send this to.
Ed> drivers/net/acenic.c: extra semicolon
Ed> near line 1090 if (!(ap->trace_buf = kmalloc(ACE_TRACE_SIZE,
Ed> GFP_KERNEL)));
Neato
The code in question would never be run by any ordinary user since it
was surrounded by a #if 0/#endif pair, but I'll fix it in the next
release anyway.
Jes
Peter T. Breuer
> On Wed, 23 Feb 2000 alme...@lrc.di.epfl.ch wrote:
>
> > > drivers/atm/eni.c: comparison takes precedence over assignment near line
> > > 1041
> > > if (!(aal5 = vcc->qos.aal == ATM_AAL5))
> > No problem here.
>
> If aal5 really is supposed to be a truth value (as the use of ! suggests),
> it would be really nice if someone would put in some parentheses.
It would be even nicer if someone wrote
aal5 = (vcc->qos.aal == ATM_AAL5);
if (!aal5) { ...
or even used a switch, since it's that kind of thing here. Maybe it's
just me, but this routine has if(aal5) all the way through, and should
probably be split into two.
Peter
alme...@lrc.di.epfl.ch
> 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
to understand, it doesn't help to give them examples where precedence is
enforced by redundant parentheses as if to suggest they were necessary,
particularly if the intended meaning is obvious (which, I agree, may not
be the case in the original code).
> 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.
> 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.
The driver is complex enough already ...
- Werner
--
_________________________________________________________________________
/ Werner Almesberger, ICA, EPFL, CH werner.al...@ica.epfl.ch /
/_IN_N_032__Tel_+41_21_693_6621__Fax_+41_21_693_6610_____________________/
-
Peter T. Breuer
"A month of sundays ago alme...@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
Jamie Lokier
> Oh, this is funny!
>
> (yes, I reorderd them)
> ...
> >drivers/scsi/53c7,8xx.c: extra semicolon near line 3152
> > for (bp = (struct NCR53c7x0_break *) host->breakpoints;
>
> Full context:
> for (bp = (struct NCR53c7x0_break *) host->breakpoints;
> bp; bp = (struct NCR53c7x0_break *) bp->next); {
>
> >drivers/char/joystick/joy-creative.c: extra semicolon near line 69
> > for (i = 0; i < 2; i++); {
>
> *grin* Those two are just classic! It's been 6 years since I've seen such
> beautiful snafu's. (I used to teach FORTRAN labs.)
Those are obvious GCC warning material. for/while/if followed by
semicolon followed by brace.
Hmm. When GCC supports column numbers, maybe warnings could be added
ased on indentation clues too, like this which I've seen in the kernel
on occasion:
if (foo);
bar;
-- Jamie
alme...@lrc.di.epfl.ch
>> Then you'd also duplicate a lot of other, common, and quite hairy code.
>
> Which should then be put into functional subunits?
Most of them would be just one or two lines, and probably harder to
understand than the original code. Also, they all share a few common
variables (e.g. dma_wr, size), so you'd end up with a comparably fat
interface and/or you'd give gcc's ability to merge common expressions
in inlined functions a pretty good test.
> I already spent a couple of years (back a couple of years) retrofitting
> atm codes into 2.0.*! I know.
Oh, I didn't expect it to be _that_ bad :-(
- Werner
--
_________________________________________________________________________
/ Werner Almesberger, ICA, EPFL, CH werner.al...@ica.epfl.ch /
/_IN_N_032__Tel_+41_21_693_6621__Fax_+41_21_693_6610_____________________/
-
natha...@amd.com
> -----Original Message-----
> From: Peter T. Breuer [mailto:p...@it.uc3m.es]
> Sent: Wednesday, February 23, 2000 8:24 AM
> To: alme...@lrc.di.epfl.ch
> Cc: linux kernel
> Subject: Re: Kernel bugs found using inspect tool
> 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.
>
With out-of-order execution, note that if((a == b) | (c == d) | (e < f))
runs faster if c, d, e, and f have no side effects--and the parens ARE
required.
Nathan
Peter T. Breuer
> > 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.
> >
>
> With out-of-order execution, note that if((a == b) | (c == d) | (e < f))
> runs faster if c, d, e, and f have no side effects--and the parens ARE
> required.
Eh? But you are using bitwise or. I had logical or, whose order is
defined, being that of lazy left-right evaluation, and whose precedence
is looser than that of the relational operators precisely so that you
can write boolean propositional compounds without those annoying parens.
Or am I wrong? It's been at least 15 years since I read K&R ...
Nah. No chance.
Peter
Alan Cox
> defined, being that of lazy left-right evaluation, and whose precedence
> is looser than that of the relational operators precisely so that you
> can write boolean propositional compounds without those annoying parens.
Thats why its faster using bitwise or. Logical or has a strict left->right
evaluation rule in C bitwise doesnt so you can generate nicer code, tricks
like near branchless and sequences using cmp, adc
Peter T. Breuer
[Charset iso-8859-1 unsupported, filtering to ASCII...]
> The bitwise ors allow the equality tests to proceed in parallel, and without
> branch aborts from mispredictions. If the evaluations are short and hard to
> predict, the bitwise version will run faster.
Oh, I see. We are talking about different things. Yes, if all your
tests produce 1 or zero, parallel (bitwise) evaluation of all the
terms in a disjunction might well be faster than (on average)
lazily evaluating half of them in order before coming up trumps.
However, in my opinion that should be left to the optimizer to decide.
Or just possibly C is missing a boolean type.
Peter
Ricky Beam
>Those are obvious GCC warning material. for/while/if followed by
>semicolon followed by brace.
>
>Hmm. When GCC supports column numbers, maybe warnings could be added
>ased on indentation clues too, like this which I've seen in the kernel
>on occasion:
>
> if (foo);
> bar;
Those are perfectly legal constructions. The compiler, while certainly
magic, isn't a psychic.
--Ricky
Jamie Lokier
> Nathan Zook <natha...@amd.com> wrote:
> > With out-of-order execution, note that if((a == b) | (c == d) | (e < f))
> > runs faster if c, d, e, and f have no side effects--and the parens ARE
> > required.
>
> defined, being that of lazy left-right evaluation, and whose precedence
> is looser than that of the relational operators precisely so that you
> can write boolean propositional compounds without those annoying parens.
The point is that an out-of-order processor (such as a modern x86)
doesn't execute instructions in the order emitted by the C compiler. So
in the above expression, it will typically fetch a, b, c, d, e and f,
and execute all the comparisons in parallel.
However, an out of order process will speculatively execute past
branches, so that || version might be just as fast. And if the branch
predictor is doing a good job, it might just be faster because you don't
issue the unlikely instructions. But Nathan is from AMD so I'm sure
he's right :-)
> Or am I wrong? It's been at least 15 years since I read K&R ...
> Nah. No chance.
Your C is fine.
enjoy,
-- Jamie
Mike Phillips
hard IRQ warning messages.
Mike Phillips
Linux Token Ring Project
http://www.linuxtr.net
Jamie Lokier
> On Wed, 23 Feb 2000, Jamie Lokier wrote:
> >Those are obvious GCC warning material. for/while/if followed by
> >semicolon followed by brace.
> >
> >Hmm. When GCC supports column numbers, maybe warnings could be added
> >ased on indentation clues too, like this which I've seen in the kernel
> >on occasion:
> >
> > if (foo);
> > bar;
>
> Those are perfectly legal constructions. The compiler, while certainly
> magic, isn't a psychic.
Yes they are legal. And in every case where I've seen them, they're a
mistake. The semicolon ones are good at being invisible too. I've
done it and spent hours not seeing the semicolon in my code. Of course
it's easy to see in someone else's code... :-)
And yes, I've seen kernel patches fixing these bugs from time to time.
That's why we have warnings. If the compiler knew they were illegal
constructions, they'd be errors...
have a nice day,
natha...@amd.com
branch(es). A branch which goes rlrlrlrlrlrlrlr will, under most schemes in
use, be mispredicted either 50 or 100% of the time. rrllrrllrrllrrllrrll is
another really bad sequence.
A REALLY good optimizer might take the lazy-branch version of the code,
recognize that the later tests are side effect-free, and chose the version
is thinks is better. Problem is, such a compiler would likely be helpless
attempting to recognize an alternating branch point such as above.
Each mispredict will cost you several clocks, depending on the processor.
Each processor has its own prediction scheme.
Which gets us to a Dykstra quote, "Programming is, by definition, what you
have to tell the computer to do."
> From: Jamie Lokier [mailto:l...@tantalophile.demon.co.uk]
> ...
> The point is that an out-of-order processor (such as a modern x86)
> doesn't execute instructions in the order emitted by the C
> compiler. So
> in the above expression, it will typically fetch a, b, c, d, e and f,
> and execute all the comparisons in parallel.
>
> However, an out of order process will speculatively execute past
> branches, so that || version might be just as fast. And if the branch
> predictor is doing a good job, it might just be faster
> because you don't
> issue the unlikely instructions. But Nathan is from AMD so I'm sure
> he's right :-)
I'm right when I say "The rules have changed. Consider bit-logicals. Time
your critical inner loops". It all depends on the exact details of the code
& the processor. When c, d, e, and f are simple variables and the first two
branches are hard to predict, bit-ors should be faster on all the latest
superscalar systems.
Nathan Zook
Jamie Lokier
> The speed trade-off is directly related to the predictability of the
> branch(es). A branch which goes rlrlrlrlrlrlrlr will, under most schemes in
> use, be mispredicted either 50 or 100% of the time. rrllrrllrrllrrllrrll is
> another really bad sequence.
I think the PII can match both of those correctly.
> A REALLY good optimizer might take the lazy-branch version of the code,
> recognize that the later tests are side effect-free, and chose the version
> is thinks is better. Problem is, such a compiler would likely be helpless
> attempting to recognize an alternating branch point such as above.
A really good optimiser might expand the control flow graph to
accomodate poor branch prediction.... but not in this example.
> I'm right when I say "The rules have changed. Consider bit-logicals. Time
> your critical inner loops". It all depends on the exact details of the code
> & the processor. When c, d, e, and f are simple variables and the first two
> branches are hard to predict, bit-ors should be faster on all the latest
> superscalar systems.
Actually bit-ors were faster for some simpler operations on old
processors too.
Consider:
mov var1,%reg
or var2,%reg
jne label
vs.
mov var1,%reg
jne label
mov var2,%reg
jne label
or even
mov var1,%reg
or %reg,%reg
jne label
mov var2,%reg
or %reg,%reg
jne label
enjoy,
-- jamie
Q
>
> > 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.
> >
>
> runs faster if c, d, e, and f have no side effects--and the parens ARE
> required.
First of all, you should let the compiler decide which way is the
fastest/best.
Second, a | is not a sequence point, while || is. The expression can
depend on that. It could be that if (a == b), that you should not use c.
It will also do all three, while one can be more then enough.
And last, there is such a thing as branch prediction, and I have no idea
which would be "better" for it :)
I also did some little test here comparing both, and the one with | seems
to be slower when using no optimization because it needs to do more
instructions. They got the same result using -O2, if only the third is
true. But then again, what do those stupid tests show.
Q
Stephen Satchell
> > > if (foo);
> > > bar;
> >
> > Those are perfectly legal constructions. The compiler, while certainly
> > magic, isn't a psychic.
>
>Yes they are legal. And in every case where I've seen them, they're a
>mistake. The semicolon ones are good at being invisible too. I've
>done it and spent hours not seeing the semicolon in my code. Of course
>it's easy to see in someone else's code... :-)
>
>And yes, I've seen kernel patches fixing these bugs from time to time.
I get burned on this from time to time, especially after working on PL/I
code. In my programs, I have a style rule that says that an empty
statement of a for, while, do, or if statement must be coded as {} in order
to show that the set of instructions to execute is empty. (I also have the
style rule that EVERY for, while, do, or if statement must have the object
instruction(s) enclosed in braces -- I got this from Plauger's book and it
has helped immensely.)
I'm looking at "indent" to see if it can recognize empty statements and
convert the lone semicolon to {} so that this sort of thing becomes more
obvious.
Linus would probably hate that...
Satch
H. Peter Anvin
> > bar;
>
> Those are perfectly legal constructions. The compiler, while certainly
> magic, isn't a psychic.
>
Legal, yes. It's also useless (if you really meant it, you ought to
write "foo; bar;" which is equivalent.) Therefore, it should be fixed
one way or the other. Unlike a for or while statement with an empty
clause -- which sometimes have legitimate uses -- an if statement with
an empty clause is pretty much useless. The only case I've seen which
makes sense is if you have something like:
if (foo)
; /* Do nothing */
else
bar;
Note that I *highly* recommend having a comment for this case, to
indicate that there really is no mistake. Same with /* fallthrough */
in switch statements.
I actually would recommend using { } instead of ; as well.
-hpa
--
<h...@transmeta.com> at work, <h...@zytor.com> in private!
"Unix gives you enough rope to shoot yourself in the foot."
-
Stephen Satchell
>Thats why its faster using bitwise or. Logical or has a strict left->right
>evaluation rule in C bitwise doesnt so you can generate nicer code, tricks
>like near branchless and sequences using cmp, adc
When I was writing a modem testing tool, I had a situation where I needed
to reduce the code run time as much as possible on x86 architecture. So
for that application I coded up the tests using logical OR and AND, and
bitwise OR and AND, and profiled the difference.
The bitwise-operations version ran about 40 percent faster than its
logical-operations equivalent. Part of the savings was the parallel
testing of multiple bits (this is looking at the modem control register of
a UART), but part of the savings was just in the sheer amount of code that
had to be fetched and executed.
No hand-tweaking of the code was necessary -- the output of the compiler
was plenty good for my purposes.
Satch
Folkert van Heusden
> I get burned on this from time to time, especially after working on PL/I
> code. In my programs, I have a style rule that says that an empty
> statement of a for, while, do, or if statement must be coded as {} in order
> to show that the set of instructions to execute is empty. (I also have the
> style rule that EVERY for, while, do, or if statement must have the object
> instruction(s) enclosed in braces -- I got this from Plauger's book and it
> has helped immensely.)
I totally agree with you.
Maybe yeah, it's more keystrokes. Yeah, code looks larger'n'stuff.
But what matters in big projects such as the linux-kernel; it must be as
maintainable as possible! So any codingstyle that makes things more
readable/
less prone to errors should be encouraged.
In my opinion, the kernel is also rather under-documented, so
to say.
--
------------------------------------------------------------
Folkert van Heusden
http://www.vanheusden.com/
some e-mail addresses: f.v.h...@ftr.nl, flo...@dds.nl
mobile phone: +31-6-22390057
Ricky Beam
>I totally agree with you.
syntaxtically correct either way.)
>Maybe yeah, it's more keystrokes. Yeah, code looks larger'n'stuff.
>But what matters in big projects such as the linux-kernel; it must be as
>maintainable as possible! So any codingstyle that makes things more
>readable less prone to errors should be encouraged.
Indeed... the kernel is already >80MB. So, what's a few curly-braces
here and there :-)
>In my opinion, the kernel is also rather under-documented, so
>to say.
"The code is self documenting." -- The Programmer.
"I don't read C." -- The Tech Writer.
--Ricky
PS: I work for a company that makes network modeling software, so I hear the
documentation wars all the time. In some cases, the docs from the
programmers (multi-phd'd nuts) can be less readable than the C++ code.
Mikael Pettersson
> On Tue, 22 Feb 2000, Rupp, Ed J wrote:
> >the AT&T toolchest called 'inspect'. I found 10 near-certain
> >bugs which may or may not be causing any observable problems
> >and 13 probable bugs which I couldn't immediately tell if they
> ...
> >===== Questionable ====
> >arch/ppc/math-emu/op-4.h: comparison takes precedence over assignment near
> >line 215
> > (x3 += ((x2 += ((x1 += ((x0 += i) < x0)) < x1) < x2)))
This whole expression is broken.
The evaluation order for the "<" operator is unspecified, which means that
subexpressions like "((x0 += i) < x0)" do not have well-defined values.
They also violate the ANSI-C rules about permissible accesses and updates
of variables between sequence points.
/Mikael
Jeffrey B. Siegal
> your critical inner loops". It all depends on the exact details of the code
> & the processor. When c, d, e, and f are simple variables and the first two
> branches are hard to predict, bit-ors should be faster on all the latest
> superscalar systems.
If c, d, e and f are simple variables, then the compiler should be able to
tell that it makes no difference whatsoever whether | or || is used, and
should generate the same (fastest) code for both.
Unfortunately, it doesn't look like gcc (as of egcs 2.91.66) is up to the
task...
Ben Kosse
> > > if((a == b) | (c == d) | (e < f))
> >
> > I'm right when I say "The rules have changed. Consider bit-logicals. Time
> > your critical inner loops". It all depends on the exact details of the code
> > & the processor. When c, d, e, and f are simple variables and the first two
> > branches are hard to predict, bit-ors should be faster on all the latest
> > superscalar systems.
> If c, d, e and f are simple variables, then the compiler should be able to
> tell that it makes no difference whatsoever whether | or || is used, and
> should generate the same (fastest) code for both.
Not necessarily true. || has the side effect of allowing you to jump out after
the first comparison. | *REQUIRES* you do all comparisons.
--
Ben Kosse <bko...@iname.com>
Jeffrey B. Siegal
> > tell that it makes no difference whatsoever whether | or || is used, and
> > should generate the same (fastest) code for both.
>
> Not necessarily true. || has the side effect of allowing you to jump out after
> the first comparison. | *REQUIRES* you do all comparisons.
Reread carefully. If "c, d, e, and f are simple variables" then the
expression in question can not have side effects, and it makes no difference
whether all the comparisons are performed or not (nor what in order they are
performed).
Ben Kosse
>
> Ben Kosse wrote:
> >
> > > natha...@amd.com wrote:
> > > > > if((a == b) | (c == d) | (e < f))
> > > If c, d, e and f are simple variables, then the compiler should be able to
> > > tell that it makes no difference whatsoever whether | or || is used, and
> > > should generate the same (fastest) code for both.
> > Not necessarily true. || has the side effect of allowing you to jump out
> > after the first comparison. | *REQUIRES* you do all comparisons.
> Reread carefully. If "c, d, e, and f are simple variables" then the
> expression in question can not have side effects, and it makes no difference
> whether all the comparisons are performed or not (nor what in order they are
> performed).
compiler is free to use, and in fact used to be commonplace. At one point in
time, people would write the code with the most frequent occurance first and
the compiler would generate a list of cmp/jmp pairs. & vs && also creates
side-effects.
10 | 11 | 00 == 11
10 || 11 || 00 == (expression that evaluates to true)
You would be absolutely correct iff C had a one-bit boolean value, but it
doesn't, so there's always a side effect when you do bitwise operations and
you can always shortcut a logical operation (not a good idea anymore with
superscalar architectures, but it was really useful on a 386).
--
Ben Kosse <bko...@iname.com>
Jeffrey B. Siegal
> "Jeffrey B. Siegal" wrote:
> >
> > Ben Kosse wrote:
> > >
> > > > natha...@amd.com wrote:
> > > > > > if((a == b) | (c == d) | (e < f))
> > > > If c, d, e and f are simple variables, then the compiler should be able to
> > > > tell that it makes no difference whatsoever whether | or || is used, and
> > > > should generate the same (fastest) code for both.
> > > Not necessarily true. || has the side effect of allowing you to jump out
> > > after the first comparison. | *REQUIRES* you do all comparisons.
> > Reread carefully. If "c, d, e, and f are simple variables" then the
> > expression in question can not have side effects, and it makes no difference
> > whether all the comparisons are performed or not (nor what in order they are
> > performed).
> And that's my point. The *very use* of || *creates* a side effect that the
> compiler is free to use, and in fact used to be commonplace. At one point in
> time, people would write the code with the most frequent occurance first and
> the compiler would generate a list of cmp/jmp pairs. & vs && also creates
> side-effects.
>
> 10 | 11 | 00 == 11
> 10 || 11 || 00 == (expression that evaluates to true)
Again, *please* look back at the example in question. Each operand of the
"or" operator is the result of a C comparison operator that returns either 0
or 1. In this case it makes *no difference whatsoever* whether | or || is
used. As I said, the compiler should be able to recognize that the two are
equivalent and generate the fastest code (either using conditional
short-circuiting or not, as the case may be) regardless of which "or" operator
is used.