On 06/10/14 16:06, James Bottomley wrote: > On Tue, 2014-06-10 at 13:37 +0200, Bart Van Assche wrote: >> On 06/03/14 10:58, Hannes Reinecke wrote: >>> + * Given a struct scsi_lun of: d2 04 0b 03 00 00 00 00, this function >>> + * returns the integer: 0x0b03d204 >>> + * >>> + * This encoding will return a standard integer LUN for LUNs smaller >>> + * than 256, which typically use a single level LUN structure with >>> + * addressing method 0. >>> **/ >>> u64 scsilun_to_int(struct scsi_lun *scsilun) >>> { >>> @@ -1279,7 +1280,7 @@ u64 scsilun_to_int(struct scsi_lun *scsilun) >>> >>> lun = 0; >>> for (i = 0; i < sizeof(lun); i += 2) >>> - lun = lun | (((scsilun->scsi_lun[i] << 8) | >>> + lun = lun | (((scsilun->scsi_lun[i] << ((i + 1) *8)) | >>> scsilun->scsi_lun[i + 1]) << (i * 8)); >>> return lun; >>> } >> >> The above code doesn't match the comment header. Parentheses have been >> placed such that each byte with an even index is shifted left (2*i+1)*8 >> bits instead of (i+1)*8. > > I don't see that in the code, which parentheses do you mean? This patch should change the code into what I think was intended by the comment above scsilun_to_int(): --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1280,8 +1280,8 @@ u64 scsilun_to_int(struct scsi_lun *scsilun) lun = 0; for (i = 0; i < sizeof(lun); i += 2) - lun = lun | (((scsilun->scsi_lun[i] << ((i + 1) *8)) | - scsilun->scsi_lun[i + 1]) << (i * 8)); + lun = lun | (((u64)scsilun->scsi_lun[i] << ((i + 1) *8)) | + ((u64)scsilun->scsi_lun[i + 1] << (i * 8))); return lun; } EXPORT_SYMBOL(scsilun_to_int); Bart. -- 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