* [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data
@ 2018-11-29 1:01 David Disseldorp
2018-11-29 1:17 ` Ly, Bryant
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: David Disseldorp @ 2018-11-29 1:01 UTC (permalink / raw)
To: target-devel
spc5r17.pdf specifies:
4.3.1 ASCII data field requirements
ASCII data fields shall contain only ASCII printable characters (i.e.,
code values 20h to 7Eh) and may be terminated with one or more ASCII
null (00h) characters.
ASCII data fields described as being left-aligned shall have any
unused bytes at the end of the field (i.e., highest offset) and the
unused bytes shall be filled with ASCII space characters (20h).
LIO currently space-pads the T10 VENDOR IDENTIFICATION and PRODUCT
IDENTIFICATION fields in the standard INQUIRY data. However, the
PRODUCT REVISION LEVEL field in the standard INQUIRY data as well as the
T10 VENDOR IDENTIFICATION field in the INQUIRY Device Identification VPD
Page are zero-terminated/zero-padded.
Fix this inconsistency by using space-padding for all of the above
fields.
Signed-off-by: David Disseldorp <ddiss@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
drivers/target/target_core_spc.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index f459118bc11b..c37dd36ec77d 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -108,12 +108,17 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
buf[7] = 0x2; /* CmdQue=1 */
- memcpy(&buf[8], "LIO-ORG ", 8);
- memset(&buf[16], 0x20, 16);
+ /*
+ * ASCII data fields described as being left-aligned shall have any
+ * unused bytes at the end of the field (i.e., highest offset) and the
+ * unused bytes shall be filled with ASCII space characters (20h).
+ */
+ memset(&buf[8], 0x20, 8 + 16 + 4);
+ memcpy(&buf[8], "LIO-ORG", sizeof("LIO-ORG") - 1);
memcpy(&buf[16], dev->t10_wwn.model,
- min_t(size_t, strlen(dev->t10_wwn.model), 16));
+ strnlen(dev->t10_wwn.model, 16));
memcpy(&buf[32], dev->t10_wwn.revision,
- min_t(size_t, strlen(dev->t10_wwn.revision), 4));
+ strnlen(dev->t10_wwn.revision, 4));
buf[4] = 31; /* Set additional length to 31 */
return 0;
@@ -251,7 +256,9 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf)
buf[off] = 0x2; /* ASCII */
buf[off+1] = 0x1; /* T10 Vendor ID */
buf[off+2] = 0x0;
- memcpy(&buf[off+4], "LIO-ORG", 8);
+ /* left align Vendor ID and pad with spaces */
+ memset(&buf[off+4], 0x20, 8);
+ memcpy(&buf[off+4], "LIO-ORG", sizeof("LIO-ORG") - 1);
/* Extra Byte for NULL Terminator */
id_len++;
/* Identifier Length */
--
2.13.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data
2018-11-29 1:01 [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
@ 2018-11-29 1:17 ` Ly, Bryant
2018-11-29 1:23 ` Lee Duncan
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Ly, Bryant @ 2018-11-29 1:17 UTC (permalink / raw)
To: target-devel
> On Nov 28, 2018, at 7:01 PM, David Disseldorp <ddiss@suse.de> wrote:
>
> spc5r17.pdf specifies:
> 4.3.1 ASCII data field requirements
> ASCII data fields shall contain only ASCII printable characters (i.e.,
> code values 20h to 7Eh) and may be terminated with one or more ASCII
> null (00h) characters.
> ASCII data fields described as being left-aligned shall have any
> unused bytes at the end of the field (i.e., highest offset) and the
> unused bytes shall be filled with ASCII space characters (20h).
>
> LIO currently space-pads the T10 VENDOR IDENTIFICATION and PRODUCT
> IDENTIFICATION fields in the standard INQUIRY data. However, the
> PRODUCT REVISION LEVEL field in the standard INQUIRY data as well as the
> T10 VENDOR IDENTIFICATION field in the INQUIRY Device Identification VPD
> Page are zero-terminated/zero-padded.
>
> Fix this inconsistency by using space-padding for all of the above
> fields.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/target/target_core_spc.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
Reviewed-by: Bryant G. Ly bly@catalogicsoftware.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data
2018-11-29 1:01 [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
2018-11-29 1:17 ` Ly, Bryant
@ 2018-11-29 1:23 ` Lee Duncan
2018-11-30 12:44 ` David Disseldorp
2018-11-30 20:02 ` Lee Duncan
3 siblings, 0 replies; 5+ messages in thread
From: Lee Duncan @ 2018-11-29 1:23 UTC (permalink / raw)
To: target-devel
On 11/28/18 5:01 PM, David Disseldorp wrote:
> spc5r17.pdf specifies:
> 4.3.1 ASCII data field requirements
> ASCII data fields shall contain only ASCII printable characters (i.e.,
> code values 20h to 7Eh) and may be terminated with one or more ASCII
> null (00h) characters.
> ASCII data fields described as being left-aligned shall have any
> unused bytes at the end of the field (i.e., highest offset) and the
> unused bytes shall be filled with ASCII space characters (20h).
>
> LIO currently space-pads the T10 VENDOR IDENTIFICATION and PRODUCT
> IDENTIFICATION fields in the standard INQUIRY data. However, the
> PRODUCT REVISION LEVEL field in the standard INQUIRY data as well as the
> T10 VENDOR IDENTIFICATION field in the INQUIRY Device Identification VPD
> Page are zero-terminated/zero-padded.
>
> Fix this inconsistency by using space-padding for all of the above
> fields.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/target/target_core_spc.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
> index f459118bc11b..c37dd36ec77d 100644
> --- a/drivers/target/target_core_spc.c
> +++ b/drivers/target/target_core_spc.c
> @@ -108,12 +108,17 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
>
> buf[7] = 0x2; /* CmdQue=1 */
>
> - memcpy(&buf[8], "LIO-ORG ", 8);
> - memset(&buf[16], 0x20, 16);
> + /*
> + * ASCII data fields described as being left-aligned shall have any
> + * unused bytes at the end of the field (i.e., highest offset) and the
> + * unused bytes shall be filled with ASCII space characters (20h).
> + */
> + memset(&buf[8], 0x20, 8 + 16 + 4);
I dislike that you are using 0x20 here (and below) instead of ' '.
> + memcpy(&buf[8], "LIO-ORG", sizeof("LIO-ORG") - 1);
> memcpy(&buf[16], dev->t10_wwn.model,
> - min_t(size_t, strlen(dev->t10_wwn.model), 16));
> + strnlen(dev->t10_wwn.model, 16));
> memcpy(&buf[32], dev->t10_wwn.revision,
> - min_t(size_t, strlen(dev->t10_wwn.revision), 4));
> + strnlen(dev->t10_wwn.revision, 4));
> buf[4] = 31; /* Set additional length to 31 */
>
> return 0;
> @@ -251,7 +256,9 @@ spc_emulate_evpd_83(struct se_cmd *cmd, unsigned char *buf)
> buf[off] = 0x2; /* ASCII */
> buf[off+1] = 0x1; /* T10 Vendor ID */
> buf[off+2] = 0x0;
> - memcpy(&buf[off+4], "LIO-ORG", 8);
> + /* left align Vendor ID and pad with spaces */
> + memset(&buf[off+4], 0x20, 8);
> + memcpy(&buf[off+4], "LIO-ORG", sizeof("LIO-ORG") - 1);
> /* Extra Byte for NULL Terminator */
> id_len++;
> /* Identifier Length */
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data
2018-11-29 1:01 [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
2018-11-29 1:17 ` Ly, Bryant
2018-11-29 1:23 ` Lee Duncan
@ 2018-11-30 12:44 ` David Disseldorp
2018-11-30 20:02 ` Lee Duncan
3 siblings, 0 replies; 5+ messages in thread
From: David Disseldorp @ 2018-11-30 12:44 UTC (permalink / raw)
To: target-devel
On Wed, 28 Nov 2018 17:23:07 -0800, Lee Duncan wrote:
> > + * unused bytes at the end of the field (i.e., highest offset) and the
> > + * unused bytes shall be filled with ASCII space characters (20h).
> > + */
> > + memset(&buf[8], 0x20, 8 + 16 + 4);
>
> I dislike that you are using 0x20 here (and below) instead of ' '.
Given that this patch already has a couple of reviewed-bys, I'd prefer
to avoid respinning it for this. Besides, I think the comment above
makes it pretty clear.
Thanks for your feedback.
Cheers, David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data
2018-11-29 1:01 [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
` (2 preceding siblings ...)
2018-11-30 12:44 ` David Disseldorp
@ 2018-11-30 20:02 ` Lee Duncan
3 siblings, 0 replies; 5+ messages in thread
From: Lee Duncan @ 2018-11-30 20:02 UTC (permalink / raw)
To: target-devel
On 11/30/18 4:44 AM, David Disseldorp wrote:
> On Wed, 28 Nov 2018 17:23:07 -0800, Lee Duncan wrote:
>
>>> + * unused bytes at the end of the field (i.e., highest offset) and the
>>> + * unused bytes shall be filled with ASCII space characters (20h).
>>> + */
>>> + memset(&buf[8], 0x20, 8 + 16 + 4);
>>
>> I dislike that you are using 0x20 here (and below) instead of ' '.
>
> Given that this patch already has a couple of reviewed-bys, I'd prefer
> to avoid respinning it for this. Besides, I think the comment above
> makes it pretty clear.
>
> Thanks for your feedback.
>
> Cheers, David
>
Also, the "0x20" is in use widely, I see, so your patch is fine with me.
Please feel free to also add my:
Reviewed-by: Lee Duncan <lduncan@suse.com>
If you wish.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-30 20:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 1:01 [PATCH v4 1/7] target: use consistent left-aligned ASCII INQUIRY data David Disseldorp
2018-11-29 1:17 ` Ly, Bryant
2018-11-29 1:23 ` Lee Duncan
2018-11-30 12:44 ` David Disseldorp
2018-11-30 20:02 ` Lee Duncan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.