>>>>> "Boaz" == Boaz Harrosh <bharrosh@xxxxxxxxxxx> writes:
Boaz> If you are already at it could you also add the things that
Boaz> bothered me? I did stare at this function before, but did not see
Boaz> the bug you are fixing, thanks. (Did see other less important
Boaz> stuff)
I'm ok with cleaning up the redundant statements even though it's a bit
of churn in what's otherwise a pure bug fix patch. Simply on the
grounds that it's silly code.
> - sa = (cdbp[8] << 8) + cdbp[9];
> + sa = be16_to_cpu(((struct scsi_varlen_cdb_hdr *)cdbp)->service_action);
Here, however, I find the original code far superior. I know what
you're doing and why but I find the old code a bazillion times easier to
read. Short and to the point. No complex typecasts and I don't have to
go hunting for varlen_cdb_hdr struct definitions. I dive straight into
SBC and see that bytes 8 and 9 in the CDB constitute the service action.
Done!
I know that the header looks the same for all variable length commands.
But we don't have a struct scsi_cdb_header with an "operation code"
entry pointing to cmnd[0] either. Nor do we have headers for the
MAINTENANCE and SERVICE ACTION commands. To me a CDB is a byte stream
and not a bunch of structs.
Anyway. I know where you stand on this :)
scsi: Fix printing of variable length commands
We dereferenced the MAINTENANCE IN array when decoding variable length
commands. Use the right array. Also consolidate identical if
statements below.
Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 9129bcf..7092ff6 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -219,18 +219,15 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)
break;
}
sa = (cdbp[8] << 8) + cdbp[9];
- name = get_sa_name(maint_in_arr, MAINT_IN_SZ, sa);
- if (name) {
+ name = get_sa_name(variable_length_arr, VARIABLE_LENGTH_SZ, sa);
+ if (name)
printk("%s", name);
- if ((cdb_len > 0) && (len != cdb_len))
- printk(", in_cdb_len=%d, ext_len=%d",
- len, cdb_len);
- } else {
+ else
printk("cdb[0]=0x%x, sa=0x%x", cdb0, sa);
- if ((cdb_len > 0) && (len != cdb_len))
- printk(", in_cdb_len=%d, ext_len=%d",
- len, cdb_len);
- }
+
+ if ((cdb_len > 0) && (len != cdb_len))
+ printk(", in_cdb_len=%d, ext_len=%d", len, cdb_len);
+
break;
case MAINTENANCE_IN:
sa = cdbp[1] & 0x1f;
--
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
