Re: [PATCH] SCSI/libsrp: fix bug in ADDITIONAL CDB LENGTH interpretation

[Matthew Wilcox <matthew@wil.cx>]

On Wed, Dec 09, 2009 at 07:52:19PM +0100, Bart Van Assche wrote:
> Fix a bug in the interpretation of the ADDITIONAL CDB LENGTH (add_cdb_len)
> field of SRP_CMD requests. According to the SRP specification, the layout
> of this single-byte field is as follows:
> * Bits 0 and 1 are reserved.
> * Bits 2 to 7 represent the ADDITIONAL CDB LENGTH field, symbolically
>   represented as n.
> * Still according to the SRP specification, the ADDITIONAL CDB section
>   takes 4*n bytes.

Your interpretation of the SRP spec does seem to be correct, and I can
totally see how the original author of this code got it wrong.

> -	offset = cmd->add_cdb_len * 4;
> +	offset = (cmd->add_cdb_len >> 2) * 4;

Would this not be better written as:

	offset = cmd->add_cdb_len & ~3;

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."