On Fri, Aug 10, 2007 at 09:03:52PM -0400, James Bottomley wrote:
> On Fri, 2007-08-10 at 14:50 -0700, akpm@xxxxxxxxxxxxxxxxxxxx wrote:
> > From: Adrian Bunk <bunk@xxxxxxxxx>
> >
> > The Coverity checker noted that we'll anyway Oops later when we ran into
> > this condition - and the error check didn't prevent that.
> >
> > Considering that the error condition shouldn't be possible, and we are
> > not able to handle it easily, this patch simply removes the pointless
> > error check.
> >
> > Signed-off-by: Adrian Bunk <bunk@xxxxxxxxx>
> > Cc: <yokota@xxxxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > ---
> >
> > drivers/scsi/nsp32.c | 4 ----
> > 1 files changed, 4 deletions(-)
> >
> > diff -puN drivers/scsi/nsp32.c~nsp32_restart_autoscsi-remove-error-check drivers/scsi/nsp32.c
> > --- a/drivers/scsi/nsp32.c~nsp32_restart_autoscsi-remove-error-check
> > +++ a/drivers/scsi/nsp32.c
> > @@ -1905,10 +1905,6 @@ static void nsp32_restart_autoscsi(struc
> >
> > nsp32_dbg(NSP32_DEBUG_RESTART, "enter");
> >
> > - if (data->cur_target == NULL || data->cur_lunt == NULL) {
> > - nsp32_msg(KERN_ERR, "Target or Lun is invalid");
> > - }
> > -
>
> I disagree pretty strongly with this ... you're not removing an error
> check, you're removing a warning printk. Before the driver would say
> what the problem is and oops. After, it will oops and you won't
> necessarily know why. I think that's a retrograde step.
"Target or Lun is invalid" isn't a precise problem description either.
But the main point is:
An Oops indicates a programming error in the kernel (or kernel
corruption), and for this purpose an Oops is a good error message.
We can't put printk()'s in front of all pointer dereferences in the
kernel.
Is there any legal way how any of these could be NULL at this point?
My impression was there isn't.
But if there is we don't need only a printk(), we'd need an error
handling that returns from the function without Oops'ing plus an error
handling in the caller.
> James
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
