From: Ankit Jain <jankit@suse.de>
To: Andy Grover <agrover@redhat.com>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] iscsi: Use struct scsi_lun in iscsi structs instead of u8[8]
Date: Fri, 17 Jun 2011 11:27:06 +0530 [thread overview]
Message-ID: <4DFAECB2.2010107@suse.de> (raw)
In-Reply-To: <1308265029-32337-1-git-send-email-agrover@redhat.com>
On 06/17/2011 04:27 AM, Andy Grover wrote:
> struct scsi_lun is also just a struct with an array of 8 octets (64 bits)
> but using it instead in iscsi structs lets us call scsilun_to_int
> without a cast, and also lets us copy it using assignment, instead of
> memcpy().
>
> Signed-off-by: Andy Grover <agrover@redhat.com>
> ---
> drivers/scsi/be2iscsi/be_main.c | 5 ++---
> drivers/scsi/bnx2i/bnx2i_hwi.c | 8 ++++----
> drivers/scsi/libiscsi.c | 14 +++++++-------
> include/scsi/iscsi_proto.h | 18 +++++++++---------
> include/scsi/libiscsi.h | 2 +-
> 5 files changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 94b9a07..96d7d07 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -3963,11 +3963,10 @@ static int beiscsi_iotask(struct iscsi_task *task, struct scatterlist *sg,
> }
> memcpy(&io_task->cmd_bhs->iscsi_data_pdu.
> dw[offsetof(struct amap_pdu_data_out, lun) / 32],
> - io_task->cmd_bhs->iscsi_hdr.lun, sizeof(struct scsi_lun));
> + &io_task->cmd_bhs->iscsi_hdr.lun, sizeof(struct scsi_lun));
>
> AMAP_SET_BITS(struct amap_iscsi_wrb, lun, pwrb,
> - cpu_to_be16((unsigned short)io_task->cmd_bhs->iscsi_hdr.
> - lun[0]));
> + cpu_to_be16(*(unsigned short*)&io_task->cmd_bhs->iscsi_hdr.lun));
> AMAP_SET_BITS(struct amap_iscsi_wrb, r2t_exp_dtl, pwrb, xferlen);
> AMAP_SET_BITS(struct amap_iscsi_wrb, wrb_idx, pwrb,
> io_task->pwrb_handle->wrb_index);
> diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c
> index 5c54a2d..550e6c4 100644
> --- 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.
--
Ankit Jain
SUSE Labs
next prev parent reply other threads:[~2011-06-17 5:55 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 [this message]
2011-06-17 12:38 ` Stefan Richter
2011-06-17 16:31 ` Andy Grover
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=4DFAECB2.2010107@suse.de \
--to=jankit@suse.de \
--cc=agrover@redhat.com \
--cc=linux-scsi@vger.kernel.org \
/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