From: Andy Grover <agrover@redhat.com>
To: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Ankit Jain <jankit@suse.de>,
linux-scsi@vger.kernel.org, Eddie Wai <eddie.wai@broadcom.com>
Subject: Re: [PATCH] iscsi: Use struct scsi_lun in iscsi structs instead of u8[8]
Date: Fri, 17 Jun 2011 09:31:38 -0700 [thread overview]
Message-ID: <4DFB816A.2020901@redhat.com> (raw)
In-Reply-To: <20110617143807.485b7e35@stein>
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
prev parent reply other threads:[~2011-06-17 16:31 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-16 22:57 [PATCH] iscsi: Use struct scsi_lun in iscsi structs instead of u8[8] Andy Grover
2011-06-17 5:57 ` Ankit Jain
2011-06-17 12:38 ` Stefan Richter
2011-06-17 16:31 ` Andy Grover [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DFB816A.2020901@redhat.com \
--to=agrover@redhat.com \
--cc=eddie.wai@broadcom.com \
--cc=jankit@suse.de \
--cc=linux-scsi@vger.kernel.org \
--cc=stefanr@s5r6.in-berlin.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox