* [PATCH] scsi: mark expected switch fall-throughs
@ 2018-10-03 14:55 Gustavo A. R. Silva
[not found] ` <BYAPR11MB2855F7449E2A15FDB28EC5B7E1E90@BYAPR11MB2855.namprd11.prod.outlook.com>
2018-10-11 2:47 ` Martin K. Petersen
0 siblings, 2 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-03 14:55 UTC (permalink / raw)
To: Don Brace, James E.J. Bottomley, Martin K. Petersen,
Adaptec OEM Raid Solutions, Willem Riede, Kai Mäkisara
Cc: esc.storagedev, linux-scsi, linux-kernel, osst-users,
Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Addresses-Coverity-ID: 1195463 ("Missing break in switch")
Addresses-Coverity-ID: 1195464 ("Missing break in switch")
Addresses-Coverity-ID: 1195465 ("Missing break in switch")
Addresses-Coverity-ID: 1195466 ("Missing break in switch")
Addresses-Coverity-ID: 1357338 ("Missing break in switch")
Addresses-Coverity-ID: 114973 ("Missing break in switch")
Addresses-Coverity-ID: 114983 ("Missing break in switch")
Addresses-Coverity-ID: 114984 ("Missing break in switch")
Addresses-Coverity-ID: 114985 ("Missing break in switch")
Addresses-Coverity-ID: 114988 ("Missing break in switch")
Addresses-Coverity-ID: 114994 ("Missing break in switch")
Addresses-Coverity-ID: 114995 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/scsi/hpsa.c | 5 +++++
drivers/scsi/ips.c | 1 +
drivers/scsi/osst.c | 6 ++++++
drivers/scsi/ppa.c | 1 +
drivers/scsi/st.c | 4 ++++
5 files changed, 17 insertions(+)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index f903983..fb6a356 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4662,6 +4662,7 @@ static int fixup_ioaccel_cdb(u8 *cdb, int *cdb_len)
case WRITE_6:
case WRITE_12:
is_write = 1;
+ /* fall through */
case READ_6:
case READ_12:
if (*cdb_len == 6) {
@@ -5092,6 +5093,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
switch (cmd->cmnd[0]) {
case WRITE_6:
is_write = 1;
+ /* fall through */
case READ_6:
first_block = (((cmd->cmnd[1] & 0x1F) << 16) |
(cmd->cmnd[2] << 8) |
@@ -5102,6 +5104,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
break;
case WRITE_10:
is_write = 1;
+ /* fall through */
case READ_10:
first_block =
(((u64) cmd->cmnd[2]) << 24) |
@@ -5114,6 +5117,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
break;
case WRITE_12:
is_write = 1;
+ /* fall through */
case READ_12:
first_block =
(((u64) cmd->cmnd[2]) << 24) |
@@ -5128,6 +5132,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
break;
case WRITE_16:
is_write = 1;
+ /* fall through */
case READ_16:
first_block =
(((u64) cmd->cmnd[2]) << 56) |
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index bd6ac6b..8e1c45d 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -3485,6 +3485,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)
case START_STOP:
scb->scsi_cmd->result = DID_OK << 16;
+ /* fall through */
case TEST_UNIT_READY:
case INQUIRY:
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 7a1a1ed..8af608d 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -216,12 +216,14 @@ static void osst_analyze_sense(struct osst_request *SRpnt, struct st_cmdstatus *
switch (sense[0] & 0x7f) {
case 0x71:
s->deferred = 1;
+ /* fall through */
case 0x70:
s->fixed_format = 1;
s->flags = sense[2] & 0xe0;
break;
case 0x73:
s->deferred = 1;
+ /* fall through */
case 0x72:
s->fixed_format = 0;
ucp = scsi_sense_desc_find(sense, SCSI_SENSE_BUFFERSIZE, 4);
@@ -591,6 +593,7 @@ static void osst_init_aux(struct osst_tape * STp, int frame_type, int frame_seq_
dat->dat_list[0].flags = frame_type==OS_FRAME_TYPE_MARKER?
OS_DAT_FLAGS_MARK:OS_DAT_FLAGS_DATA;
dat->dat_list[0].reserved = 0;
+ /* fall through */
case OS_FRAME_TYPE_EOD:
aux->update_frame_cntr = htonl(0);
par->partition_num = OS_DATA_PARTITION;
@@ -4086,6 +4089,7 @@ static int osst_int_ioctl(struct osst_tape * STp, struct osst_request ** aSRpnt,
switch (cmd_in) {
case MTFSFM:
chg_eof = 0; /* Changed from the FSF after this */
+ /* fall through */
case MTFSF:
if (STp->raw)
return (-EIO);
@@ -4101,6 +4105,7 @@ static int osst_int_ioctl(struct osst_tape * STp, struct osst_request ** aSRpnt,
case MTBSF:
chg_eof = 0; /* Changed from the FSF after this */
+ /* fall through */
case MTBSFM:
if (STp->raw)
return (-EIO);
@@ -4312,6 +4317,7 @@ static int osst_int_ioctl(struct osst_tape * STp, struct osst_request ** aSRpnt,
name, STp->block_size);
return 0;
}
+ /* fall through */
case MTSETDENSITY: /* Set tape density */
case MTSETDRVBUFFER: /* Set drive buffering */
case SET_DENS_AND_BLK: /* Set density and block size */
diff --git a/drivers/scsi/ppa.c b/drivers/scsi/ppa.c
index ee86a0c..d29999b 100644
--- a/drivers/scsi/ppa.c
+++ b/drivers/scsi/ppa.c
@@ -717,6 +717,7 @@ static int ppa_engine(ppa_struct *dev, struct scsi_cmnd *cmd)
}
cmd->SCp.phase++;
}
+ /* fall through */
case 2: /* Phase 2 - We are now talking to the scsi bus */
if (!ppa_select(dev, scmd_id(cmd))) {
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 307df2f..f1351ec 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -337,12 +337,14 @@ static void st_analyze_sense(struct st_request *SRpnt, struct st_cmdstatus *s)
switch (sense[0] & 0x7f) {
case 0x71:
s->deferred = 1;
+ /* fall through */
case 0x70:
s->fixed_format = 1;
s->flags = sense[2] & 0xe0;
break;
case 0x73:
s->deferred = 1;
+ /* fall through */
case 0x72:
s->fixed_format = 0;
ucp = scsi_sense_desc_find(sense, SCSI_SENSE_BUFFERSIZE, 4);
@@ -2721,6 +2723,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned int cmd_in, unsigned lon
switch (cmd_in) {
case MTFSFM:
chg_eof = 0; /* Changed from the FSF after this */
+ /* fall through */
case MTFSF:
cmd[0] = SPACE;
cmd[1] = 0x01; /* Space FileMarks */
@@ -2735,6 +2738,7 @@ static int st_int_ioctl(struct scsi_tape *STp, unsigned int cmd_in, unsigned lon
break;
case MTBSFM:
chg_eof = 0; /* Changed from the FSF after this */
+ /* fall through */
case MTBSF:
cmd[0] = SPACE;
cmd[1] = 0x01; /* Space FileMarks */
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mark expected switch fall-throughs
[not found] ` <BYAPR11MB2855F7449E2A15FDB28EC5B7E1E90@BYAPR11MB2855.namprd11.prod.outlook.com>
@ 2018-10-03 15:47 ` Gustavo A. R. Silva
0 siblings, 0 replies; 5+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-03 15:47 UTC (permalink / raw)
To: Don.Brace, don.brace, jejb, martin.petersen, aacraid, osst, Kai.Makisara
Cc: esc.storagedev, linux-scsi, linux-kernel, osst-users
On 10/3/18 5:44 PM, Don.Brace@microchip.com wrote:
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> *From:*Gustavo A. R. Silva <gustavo@embeddedor.com>
> *Sent:* Wednesday, October 3, 2018 9:55 AM
> *To:* Don Brace; James E.J. Bottomley; Martin K. Petersen; Adaptec OEM Raid Solutions; Willem Riede; Kai Mäkisara
> *Cc:* esc.storagedev@microsemi.com; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; osst-users@lists.sourceforge.net; Gustavo A. R. Silva
> *Subject:* [PATCH] scsi: mark expected switch fall-throughs
>
>
>
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/scsi/hpsa.c | 5 +++++
> drivers/scsi/ips.c | 1 +
> drivers/scsi/osst.c | 6 ++++++
> drivers/scsi/ppa.c | 1 +
> drivers/scsi/st.c | 4 ++++
> 5 files changed, 17 insertions(+)
>
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index f903983..fb6a356 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -4662,6 +4662,7 @@ static int fixup_ioaccel_cdb(u8 *cdb, int *cdb_len)
> case WRITE_6:
> case WRITE_12:
> is_write = 1;
> + /* fall through */
> case READ_6:
> case READ_12:
> if (*cdb_len == 6) {
> @@ -5092,6 +5093,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
> switch (cmd->cmnd[0]) {
> case WRITE_6:
> is_write = 1;
> + /* fall through */
> case READ_6:
> first_block = (((cmd->cmnd[1] & 0x1F) << 16) |
> (cmd->cmnd[2] << 8) |
> @@ -5102,6 +5104,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
> break;
> case WRITE_10:
> is_write = 1;
> + /* fall through */
> case READ_10:
> first_block =
> (((u64) cmd->cmnd[2]) << 24) |
> @@ -5114,6 +5117,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
> break;
> case WRITE_12:
> is_write = 1;
> + /* fall through */
> case READ_12:
> first_block =
> (((u64) cmd->cmnd[2]) << 24) |
> @@ -5128,6 +5132,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
> break;
> case WRITE_16:
> is_write = 1;
> + /* fall through */
> case READ_16:
> first_block =
> (((u64) cmd->cmnd[2]) << 56) |
>
>
>
> Tested-by: Don Brace <don.brace@microsemi.com>
> Acked-by: Don Brace <don.brace@microsemi.com>
>
Thanks for this, but please don't remove the Coverity tags.
--
Gustavo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mark expected switch fall-throughs
2018-10-03 14:55 [PATCH] scsi: mark expected switch fall-throughs Gustavo A. R. Silva
[not found] ` <BYAPR11MB2855F7449E2A15FDB28EC5B7E1E90@BYAPR11MB2855.namprd11.prod.outlook.com>
@ 2018-10-11 2:47 ` Martin K. Petersen
2018-10-12 12:26 ` Gustavo A. R. Silva
1 sibling, 1 reply; 5+ messages in thread
From: Martin K. Petersen @ 2018-10-11 2:47 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Don Brace, James E.J. Bottomley, Martin K. Petersen,
Adaptec OEM Raid Solutions, Willem Riede, Kai Mäkisara,
esc.storagedev, linux-scsi, linux-kernel, osst-users
Hi Gustavo,
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
I'm not entirely convinced that all these identified fall through cases
are intentional. From a quick glance, some of them look like bugs...
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index bd6ac6b..8e1c45d 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -3485,6 +3485,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)
>
> case START_STOP:
> scb->scsi_cmd->result = DID_OK << 16;
> + /* fall through */
>
> case TEST_UNIT_READY:
> case INQUIRY:
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mark expected switch fall-throughs
2018-10-11 2:47 ` Martin K. Petersen
@ 2018-10-12 12:26 ` Gustavo A. R. Silva
2018-10-16 2:47 ` Martin K. Petersen
0 siblings, 1 reply; 5+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-12 12:26 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Don Brace, James E.J. Bottomley, Adaptec OEM Raid Solutions,
Willem Riede, Kai Mäkisara, esc.storagedev, linux-scsi,
linux-kernel, osst-users
Hi Martin,
On 10/11/18 4:47 AM, Martin K. Petersen wrote:
>
> Hi Gustavo,
>
>> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
>> where we are expecting to fall through.
>
> I'm not entirely convinced that all these identified fall through cases
> are intentional. From a quick glance, some of them look like bugs...
>
I took a second look at this and, certainly, the one below looks more like a
bug. The rest seem to be false positives.
Also, notice that this code has been there since 2005. So, that's why I was
inclined to think that all of them are false positives.
>> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
>> index bd6ac6b..8e1c45d 100644
>> --- a/drivers/scsi/ips.c
>> +++ b/drivers/scsi/ips.c
>> @@ -3485,6 +3485,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)
>>
>> case START_STOP:
>> scb->scsi_cmd->result = DID_OK << 16;
>> + /* fall through */
If you confirm this is an actual bug, I can send a separate fix.
>>
>> case TEST_UNIT_READY:
>> case INQUIRY:
>
Thanks for the feedback.
--
Gustavo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi: mark expected switch fall-throughs
2018-10-12 12:26 ` Gustavo A. R. Silva
@ 2018-10-16 2:47 ` Martin K. Petersen
0 siblings, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2018-10-16 2:47 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Martin K. Petersen, Don Brace, James E.J. Bottomley,
Adaptec OEM Raid Solutions, Willem Riede, Kai Mäkisara,
esc.storagedev, linux-scsi, linux-kernel, osst-users
Gustavo,
>> I'm not entirely convinced that all these identified fall through cases
>> are intentional. From a quick glance, some of them look like bugs...
>
> I took a second look at this and, certainly, the one below looks more like a
> bug. The rest seem to be false positives.
Yep.
>>> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
>>> index bd6ac6b..8e1c45d 100644
>>> --- a/drivers/scsi/ips.c
>>> +++ b/drivers/scsi/ips.c
>>> @@ -3485,6 +3485,7 @@ ips_send_cmd(ips_ha_t * ha, ips_scb_t * scb)
>>>
>>> case START_STOP:
>>> scb->scsi_cmd->result = DID_OK << 16;
>>> + /* fall through */
>
> If you confirm this is an actual bug, I can send a separate fix.
I believe it is. So please do.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-16 2:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-03 14:55 [PATCH] scsi: mark expected switch fall-throughs Gustavo A. R. Silva
[not found] ` <BYAPR11MB2855F7449E2A15FDB28EC5B7E1E90@BYAPR11MB2855.namprd11.prod.outlook.com>
2018-10-03 15:47 ` Gustavo A. R. Silva
2018-10-11 2:47 ` Martin K. Petersen
2018-10-12 12:26 ` Gustavo A. R. Silva
2018-10-16 2:47 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).