Date: Tue, 29 Mar 2005 12:46:22 +0200 (CEST)
To: akpm@osdl.org
Subject: Re: Do not misuse Coverity please (Was: sound/oss/cs46xx.c: fix a check after use)
Message-ID: <xyDqcv4K.1112093182.7253990.khali@localhost>
In-Reply-To: <20050328222348.4c05e85c.akpm@osdl.org>
From: "Jean Delvare" <khali@linux-fr.org>
Bounce-To: "Jean Delvare" <khali@linux-fr.org>
CC: "Adrian Bunk" <bunk@stusta.de>, "LKML" <linux-kernel@vger.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7BIT
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1551
Lines: 37


Hi Andrew, all,

> >  Think about it. If the pointer could be NULL, then it's unlikely that
> >  the bug would have gone unnoticed so far (unless the code is very
> >  recent). Coverity found 3 such bugs in one i2c driver [1], and the
> >  correct solution was to NOT check for NULL because it just couldn't
> >  happen.
>
> No, there is a third case: the pointer can be NULL, but the compiler
> happened to move the dereference down to after the check.

Wow. Great point. I completely missed that possibility. In fact I didn't
know that the compiler could possibly alter the order of the
instructions. For one thing, I thought it was simply not allowed to. For
another, I didn't know that it had been made so aware that it could
actually figure out how to do this kind of things. What a mess. Let's
just hope that the gcc folks know their business :)

I'll try to remember this next time I debug something. Do not assume
that instructions are run in the order seen in the source. Irk.

> If the optimiser is later changed, or if someone tries to compile the code
> with -O0, it will oops.

Interesting. Then wouldn't it be worth attempting such compilations at
times, and see if we get additional oops? Doesn't gcc have a flag to
specifically forbid this family of optimizations?

Thanks,
--
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/