RE: [PATCH 17/24] scsi: remove last argument from print_opcode_name()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Hannes Reinecke [mailto:hare@xxxxxxx]
> Sent: Wednesday, 01 October, 2014 1:23 AM
> To: James Bottomley
> Cc: Christoph Hellwig; linux-scsi@xxxxxxxxxxxxxxx; Elliott, Robert (Server
> Storage); Hannes Reinecke
> Subject: [PATCH 17/24] scsi: remove last argument from print_opcode_name()
> 
> print_opcode_name() was only ever called with a '0' argument
> from LLDDs and ULDs which were _not_ supporting variable length
> CDBs, so the 'if' clause was never triggered.
> 
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> ---
>  drivers/scsi/constants.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index 2110d61..713e1e0 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -320,25 +320,21 @@ static bool scsi_opcode_sa_name(int cmd, int
> service_action,
>  	return true;
>  }
> 
> -/* attempt to guess cdb length if cdb_len==0 . No trailing linefeed. */
> -static void print_opcode_name(unsigned char * cdbp, int cdb_len)
> +static void print_opcode_name(unsigned char *cdbp)

Add const since what cdbp points to is not modified.

>  {
> -	int sa, len, cdb0;
> +	int sa, cdb0;
>  	const char *cdb_name = NULL, *sa_name = NULL;
> 
>  	cdb0 = cdbp[0];
>  	if (cdb0 == VARIABLE_LENGTH_CMD) {
> -		len = scsi_varlen_cdb_length(cdbp);
> +		int len = scsi_varlen_cdb_length(cdbp);

cdbp must point to a buffer containing at least 8 valid
bytes for that to be safe. Is that guaranteed?

>  		if (len < 10) {
> -			printk("short variable length command, "
> -			       "len=%d ext_len=%d", len, cdb_len);
> +			printk("short variable length command, len=%d", len);
>  			return;
>  		}
>  		sa = (cdbp[8] << 8) + cdbp[9];
> -	} else {
> +	} else
>  		sa = cdbp[1] & 0x1f;

cdbp must point to a buffer containing at least 2 valid
bytes for that to be safe. Is that guaranteed?

> -		len = cdb_len;
> -	}
> 
>  	if (!scsi_opcode_sa_name(cdb0, sa, &cdb_name, &sa_name)) {
>  		if (cdb_name)
> @@ -356,9 +352,6 @@ static void print_opcode_name(unsigned char * cdbp, int
> cdb_len)
>  			printk("%s, sa=0x%x", cdb_name, sa);
>  		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);
>  	}
>  }
> 
> @@ -366,7 +359,7 @@ void __scsi_print_command(unsigned char *cdb)
...
> @@ -383,7 +376,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)

Add const to each of those, since what cdbp and cmd point 
to is not modified.

---
Rob Elliott    HP Server Storage




--
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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux