"Jean Delvare" <[EMAIL PROTECTED]> said:
[Sttributions missing, sorry]
> > > 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
>
On Mar 29, 2005, at 09:22, Daniel Jacobowitz wrote:
The thing GCC is most likely to do with this code is discard the NULL
check entirely and leave only the oops; the "if (!card)" can not be
reached without passing through "card->amplifier", and a pointer which
is dereferenced can not be NULL in a
On Mon, Mar 28, 2005 at 10:23:48PM -0800, Andrew Morton wrote:
> > > -int old=card->amplifier;
> > > +int old;
> > > if(!card)
> > > {
> > > CS_DBGOUT(CS_ERROR, 2, printk(KERN_INFO
> > > "cs46xx: amp_hercules()
Jean Delvare wrote:
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
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
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
Jean Delvare wrote:
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
On Mon, Mar 28, 2005 at 10:23:48PM -0800, Andrew Morton wrote:
-int old=card-amplifier;
+int old;
if(!card)
{
CS_DBGOUT(CS_ERROR, 2, printk(KERN_INFO
cs46xx: amp_hercules() called before
On Mar 29, 2005, at 09:22, Daniel Jacobowitz wrote:
The thing GCC is most likely to do with this code is discard the NULL
check entirely and leave only the oops; the if (!card) can not be
reached without passing through card-amplifier, and a pointer which
is dereferenced can not be NULL in a valid
Jean Delvare [EMAIL PROTECTED] said:
[Sttributions missing, sorry]
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
Jean Delvare <[EMAIL PROTECTED]> wrote:
>
> > This patch fixes a check after use found by the Coverity checker.
> > (...)
> > static void amp_hercules(struct cs_card *card, int change)
> > {
> > - int old=card->amplifier;
> > + int old;
> >if(!card)
> >{
> >
On Mon, 28 Mar 2005, L. A. Walsh wrote:
> However, in this case, if the author is _certain_ the
> pointer can never be NULL, than an "ASSERT(card!=NULL);" might
> be appropriate, where ASSERT is a macro that normally compiles
> in the check, but could compile to "nothing" for embedded or
>
Adrian Bunk wrote:
On Sun, Mar 27, 2005 at 11:21:58PM +0200, Jean Delvare wrote:
There are two cases:
1. NULL is impossible, the check is superfluous
2. this was an actual bug
In the first case, my patch doesn't do any harm (a superfluous isn't a
real bug).
In the second case, it fixed a
Jean Delvare schrieb:
Hi Adrian,
There are two cases:
1. NULL is impossible, the check is superfluous
2. this was an actual bug
Agreed.
In the first case, my patch doesn't do any harm (a superfluous isn't
a real bug).
The fact that it isn't a bug does not imply that the patch
Jean Delvare schrieb:
Hi Adrian,
There are two cases:
1. NULL is impossible, the check is superfluous
2. this was an actual bug
Agreed.
In the first case, my patch doesn't do any harm (a superfluous isn't
a real bug).
The fact that it isn't a bug does not imply that the patch
Adrian Bunk wrote:
On Sun, Mar 27, 2005 at 11:21:58PM +0200, Jean Delvare wrote:
There are two cases:
1. NULL is impossible, the check is superfluous
2. this was an actual bug
In the first case, my patch doesn't do any harm (a superfluous isn't a
real bug).
In the second case, it fixed a
On Mon, 28 Mar 2005, L. A. Walsh wrote:
However, in this case, if the author is _certain_ the
pointer can never be NULL, than an ASSERT(card!=NULL); might
be appropriate, where ASSERT is a macro that normally compiles
in the check, but could compile to nothing for embedded or
kernels
Jean Delvare [EMAIL PROTECTED] wrote:
This patch fixes a check after use found by the Coverity checker.
(...)
static void amp_hercules(struct cs_card *card, int change)
{
- int old=card-amplifier;
+ int old;
if(!card)
{
CS_DBGOUT(CS_ERROR, 2,
On Mon, Mar 28, 2005 at 12:34:01AM +0200, Jean Delvare wrote:
> I think that you'd be better just telling the
> maintainers about the problem without providing an arbitrary patch, so
> that they will actually look into the problem with their advanced
> knowledge of the driver
FWIW, I agree with
Hi Adrian,
> There are two cases:
> 1. NULL is impossible, the check is superfluous
> 2. this was an actual bug
Agreed.
> In the first case, my patch doesn't do any harm (a superfluous isn't
> a real bug).
The fact that it isn't a bug does not imply that the patch doesn't harm.
Tricking the
On Sun, Mar 27, 2005 at 11:21:58PM +0200, Jean Delvare wrote:
> Hi Adrian,
>
> > This patch fixes a check after use found by the Coverity checker.
> > (...)
> > static void amp_hercules(struct cs_card *card, int change)
> > {
> > - int old=card->amplifier;
> > + int old;
> > if(!card)
>
Hi Adrian,
> This patch fixes a check after use found by the Coverity checker.
> (...)
> static void amp_hercules(struct cs_card *card, int change)
> {
> - int old=card->amplifier;
> + int old;
> if(!card)
> {
> CS_DBGOUT(CS_ERROR, 2, printk(KERN_INFO
>
Hi Adrian,
This patch fixes a check after use found by the Coverity checker.
(...)
static void amp_hercules(struct cs_card *card, int change)
{
- int old=card-amplifier;
+ int old;
if(!card)
{
CS_DBGOUT(CS_ERROR, 2, printk(KERN_INFO
Hi Adrian,
There are two cases:
1. NULL is impossible, the check is superfluous
2. this was an actual bug
Agreed.
In the first case, my patch doesn't do any harm (a superfluous isn't
a real bug).
The fact that it isn't a bug does not imply that the patch doesn't harm.
Tricking the reader
On Mon, Mar 28, 2005 at 12:34:01AM +0200, Jean Delvare wrote:
I think that you'd be better just telling the
maintainers about the problem without providing an arbitrary patch, so
that they will actually look into the problem with their advanced
knowledge of the driver
FWIW, I agree with Jean.
25 matches
Mail list logo