On 14-06-10 07:37 AM, 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 assume this means the parentheses have been
misplaced ? Another bug in this code is that a cast from
scsilun->scsi_lun[i] from u8 to u64 is missing. I think the shift
operations in the above code trigger what is called undefined behavior
in the C standard if i >= 4.
Hmm, we have been over this ground before. In sg_luns,
to support translating between the T10 and Linux
representation of 64 bit LUNs, there are these two
versions:
static uint64_t
t10_2linux_lun(const unsigned char t10_lun[])
{
const unsigned char * cp;
uint64_t res;
res = (t10_lun[6] << 8) + t10_lun[7];
for (cp = t10_lun + 4; cp >= t10_lun; cp -= 2) {
res <<= 16;
res += (*cp << 8) + *(cp + 1);
}
return res;
}
/* Copy of t10_lun --> Linux unsigned int (i.e. 32 bit ) present in Linux
* kernel, up to least lk 3.8.0, extended to 64 bits.
* BEWARE: for sizeof(int==4) this function is BROKEN and is left here as
* as example and may soon be removed. */
static uint64_t
t10_2linux_lun64bitBR(const unsigned char t10_lun[])
{
int i;
uint64_t lun;
lun = 0;
for (i = 0; i < (int)sizeof(lun); i += 2)
lun = lun | (((t10_lun[i] << 8) | t10_lun[i + 1]) << (i * 8));
return lun;
}
The second one (with "BR" (for broken) appended) is for testing.
And that second one looks very similar to the code you are
objecting to.
Doug Gilbert
--
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
