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.

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