Re: [PATCH] iscsi: Use struct scsi_lun in iscsi structs instead of u8[8]

[Andy Grover <agrover@redhat.com>]

On 06/17/2011 05:38 AM, Stefan Richter wrote:
> On Jun 17 Ankit Jain wrote:
>> On 06/17/2011 04:27 AM, Andy Grover wrote:
>>> --- a/drivers/scsi/bnx2i/bnx2i_hwi.c
>>> +++ b/drivers/scsi/bnx2i/bnx2i_hwi.c
>>> @@ -430,7 +430,7 @@ int bnx2i_send_iscsi_tmf(struct bnx2i_conn *bnx2i_conn,
>>>  	default:
>>>  		tmfabort_wqe->ref_itt = RESERVED_ITT;
>>>  	}
>>> -	memcpy(scsi_lun, tmfabort_hdr->lun, sizeof(struct scsi_lun));
>>> +	memcpy(scsi_lun, &tmfabort_hdr->lun, sizeof(struct scsi_lun));
>>>  	tmfabort_wqe->lun[0] = be32_to_cpu(scsi_lun[0]);
>>>  	tmfabort_wqe->lun[1] = be32_to_cpu(scsi_lun[1]);
>>>  
>>> @@ -547,7 +547,7 @@ int bnx2i_send_iscsi_nopout(struct bnx2i_conn *bnx2i_conn,
>>>  
>>>  	nopout_wqe->op_code = nopout_hdr->opcode;
>>>  	nopout_wqe->op_attr = ISCSI_FLAG_CMD_FINAL;
>>> -	memcpy(nopout_wqe->lun, nopout_hdr->lun, 8);
>>> +	memcpy(nopout_wqe->lun, &nopout_hdr->lun, 8);
>>
>> Should you be using "sizeof (..)" here (and similar instances), rather
>> than 8? It is being done that way in other instances and it would be
>> better practice, IMHO.
> 
> sizeof or not sizeof is the least of the coding style issues in drivers/scsi/bnx2i/.
> Exhibit one from 57xx_iscsi_hsi.h:

<snip>

I hear ya. The focus of this patch was really the change to
iscsi_proto.h. All the other bits are compile fixes from that, so I
tried not to get distracted by other obvious driver issues.

Regards -- Andy