Re: [PATCH] scsi: ips: fix missing break in switch

Finn Thain
2018-10-18T02:37:24Z

On Wed, 17 Oct 2018, Martin K. Petersen wrote:

>
> >> See the case statements above for another fast exit scenario.
> >>
> >
> > But that's an error path.
>
> Look further down. Several other SCSI commands are completed as NOPs the
> same way.
>

That's true, but it doesn't indicate a bug to me.

On the contrary, the entire switch (scb->scsi_cmd->cmnd[0]) statement in
ips_send_cmd() appears to be carefully constructed, just like the matching
statement in ips_chkstatus().

But none of these indications can decide the question. We just don't have
enough information. (I'm sure that someone somewhere does have the
relevant technical information...)

If this really is undecidable, then I think the right patch is the more
prudent one. That is, add a "fall through" comment, not a "break"
statement.

Or perhaps a "fall through (TODO: check this)" comment.

> Also, I don't see how the case statement for TUR/INQUIRY would do
> anything meaningful in terms of preparing a START STOP UNIT for the
> firmware.
>

If SSU case doesn't do anything meaningful, then neither does the
TUR/INQUIRY case, and then you can just delete all of that code:

} else {
scb->cmd.logical_info.op_code = IPS_CMD_GET_LD_INFO;
scb->cmd.logical_info.command_id = IPS_COMMAND_ID(ha, scb);
scb->cmd.logical_info.reserved = 0;
scb->cmd.logical_info.reserved2 = 0;
scb->data_len = sizeof (IPS_LD_INFO);
scb->data_busaddr = ha->logical_drive_info_dma_addr;
scb->flags = 0;
scb->cmd.logical_info.buffer_addr = scb->data_busaddr;
ret = IPS_SUCCESS;
}

FWIW, I think this line of reasoning is mistaken.

--