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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to