2007-10-24 16:38:50

by Adrian Bunk

[permalink] [raw]
Subject: [RFC: 2.6 patch] nsp32_restart_autoscsi(): remove error check

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 <[email protected]>

---

This patch has been sent on:
- 23 Jul 2007

drivers/scsi/nsp32.c | 4 ----
1 file changed, 4 deletions(-)

--- linux-2.6.22-rc6-mm1/drivers/scsi/nsp32.c.old 2007-07-23 19:38:58.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/scsi/nsp32.c 2007-07-23 20:17:14.000000000 +0200
@@ -1888,44 +1888,40 @@ static void nsp32_msgout_occur(struct sc

data->msgout_len = 0;

nsp32_dbg(NSP32_DEBUG_MSGOUTOCCUR, "exit");
}

/*
* Restart AutoSCSI
*
* Note: Restarting AutoSCSI needs set:
* SYNC_REG, ACK_WIDTH, SGT_ADR, TRANSFER_CONTROL
*/
static void nsp32_restart_autoscsi(struct scsi_cmnd *SCpnt, unsigned short command)
{
nsp32_hw_data *data = (nsp32_hw_data *)SCpnt->device->host->hostdata;
unsigned int base = data->BaseAddress;
unsigned short transfer = 0;

nsp32_dbg(NSP32_DEBUG_RESTART, "enter");

- if (data->cur_target == NULL || data->cur_lunt == NULL) {
- nsp32_msg(KERN_ERR, "Target or Lun is invalid");
- }
-
/*
* set SYNC_REG
* Don't set BM_START_ADR before setting this register.
*/
nsp32_write1(base, SYNC_REG, data->cur_target->syncreg);

/*
* set ACKWIDTH
*/
nsp32_write1(base, ACK_WIDTH, data->cur_target->ackwidth);

/*
* set SREQ hazard killer sampling rate
*/
nsp32_write1(base, SREQ_SMPL_RATE, data->cur_target->sample_reg);

/*
* set SGT ADDR (physical address)
*/
nsp32_write4(base, SGT_ADR, data->cur_lunt->sglun_paddr);


2007-10-24 16:57:00

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] nsp32_restart_autoscsi(): remove error check

On Wed, 2007-10-24 at 18:25 +0200, Adrian Bunk wrote:
> 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.

This is the one where I said I don't like the fact that you're removing
the explanatory message that's printed before we crash. if you want to
shut coverity up and stop sending the patch, I'm happy adding a BUG();
after the nsp32_msg().

James


2007-10-24 17:02:38

by Adrian Bunk

[permalink] [raw]
Subject: Re: [RFC: 2.6 patch] nsp32_restart_autoscsi(): remove error check

On Wed, Oct 24, 2007 at 12:41:12PM -0400, James Bottomley wrote:
> On Wed, 2007-10-24 at 18:25 +0200, Adrian Bunk wrote:
> > 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.
>
> This is the one where I said I don't like the fact that you're removing
> the explanatory message that's printed before we crash. if you want to
> shut coverity up and stop sending the patch, I'm happy adding a BUG();
> after the nsp32_msg().

Sorry, I missed your answer when resending since it was in a different
thread.

I'm still not convinced we really need the nsp32_msg() here but you are
the maintainer and it's not a big deal anyway

A BUG() would IMHO be similarly pointless as the nsp32_msg() at this
point and there's no value in changing code only for the sake of the
Coverity checker.

> 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