Re: [PATCH 3/3] scsi: varlen extended and vendor-specific cdbs

[Christoph Hellwig <hch@infradead.org>]

On Sun, Feb 10, 2008 at 09:12:02PM +0200, Boaz Harrosh wrote:
> @@ -525,6 +516,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>  	unsigned long flags = 0;
>  	unsigned long timeout;
>  	int rtn = 0;
> +	unsigned cmd_len;
>  
>  	/* check if the device is still usable */
>  	if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
> @@ -606,9 +598,17 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
>  	 * Before we queue this command, check if the command
>  	 * length exceeds what the host adapter can handle.
>  	 */
> -	if (CDB_SIZE(cmd) > cmd->device->host->max_cmd_len) {
> +	cmd_len = cmd->cmd_len;
> +	if (!cmd_len) {
> +		BUG_ON(cmd->cmnd[0] == VARIABLE_LENGTH_CMD);
> +		cmd_len = COMMAND_SIZE((cmd)->cmnd[0]);
> +	}

This looks odd to me.  Shouldn't we make sure cmd_len is always
initialized in a single place for either varlen or fixed length
commands and not have things like this?