* [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps
@ 2018-11-29 1:01 David Disseldorp
2018-11-29 1:33 ` Lee Duncan
2018-11-29 16:37 ` Bart Van Assche
0 siblings, 2 replies; 3+ messages in thread
From: David Disseldorp @ 2018-11-29 1:01 UTC (permalink / raw)
To: target-devel
From: Bart Van Assche <bvanassche@acm.org>
The existing for loops step over null-terminators for right-padding.
Padding can be achieved in a much simpler way using printf width
specifiers.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
drivers/target/target_core_device.c | 35 ++++++++---------------------------
drivers/target/target_core_stat.c | 32 +++++++-------------------------
2 files changed, 15 insertions(+), 52 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index b3d0bd1ab09f..7f2d307e411b 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -720,36 +720,17 @@ void core_dev_free_initiator_node_lun_acl(
static void scsi_dump_inquiry(struct se_device *dev)
{
struct t10_wwn *wwn = &dev->t10_wwn;
- char buf[INQUIRY_MODEL_LEN + 1];
- int i, device_type;
+ int device_type = dev->transport->get_device_type(dev);
+
/*
* Print Linux/SCSI style INQUIRY formatting to the kernel ring buffer
*/
- for (i = 0; i < INQUIRY_VENDOR_LEN; i++)
- if (wwn->vendor[i] >= 0x20)
- buf[i] = wwn->vendor[i];
- else
- buf[i] = ' ';
- buf[i] = '\0';
- pr_debug(" Vendor: %s\n", buf);
-
- for (i = 0; i < INQUIRY_MODEL_LEN; i++)
- if (wwn->model[i] >= 0x20)
- buf[i] = wwn->model[i];
- else
- buf[i] = ' ';
- buf[i] = '\0';
- pr_debug(" Model: %s\n", buf);
-
- for (i = 0; i < INQUIRY_REVISION_LEN; i++)
- if (wwn->revision[i] >= 0x20)
- buf[i] = wwn->revision[i];
- else
- buf[i] = ' ';
- buf[i] = '\0';
- pr_debug(" Revision: %s\n", buf);
-
- device_type = dev->transport->get_device_type(dev);
+ pr_debug(" Vendor: %-" __stringify(INQUIRY_VENDOR_LEN) "s\n",
+ wwn->vendor);
+ pr_debug(" Model: %-" __stringify(INQUIRY_MODEL_LEN) "s\n",
+ wwn->model);
+ pr_debug(" Revision: %-" __stringify(INQUIRY_REVISION_LEN) "s\n",
+ wwn->revision);
pr_debug(" Type: %s ", scsi_device_type(device_type));
}
diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index e437ba494865..87fd2b11fe3b 100644
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -246,43 +246,25 @@ static ssize_t target_stat_lu_lu_name_show(struct config_item *item, char *page)
static ssize_t target_stat_lu_vend_show(struct config_item *item, char *page)
{
struct se_device *dev = to_stat_lu_dev(item);
- int i;
- char str[INQUIRY_VENDOR_LEN+1];
- /* scsiLuVendorId */
- for (i = 0; i < INQUIRY_VENDOR_LEN; i++)
- str[i] = ISPRINT(dev->t10_wwn.vendor[i]) ?
- dev->t10_wwn.vendor[i] : ' ';
- str[i] = '\0';
- return snprintf(page, PAGE_SIZE, "%s\n", str);
+ return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_VENDOR_LEN)
+ "s\n", dev->t10_wwn.vendor);
}
static ssize_t target_stat_lu_prod_show(struct config_item *item, char *page)
{
struct se_device *dev = to_stat_lu_dev(item);
- int i;
- char str[INQUIRY_MODEL_LEN+1];
- /* scsiLuProductId */
- for (i = 0; i < INQUIRY_MODEL_LEN; i++)
- str[i] = ISPRINT(dev->t10_wwn.model[i]) ?
- dev->t10_wwn.model[i] : ' ';
- str[i] = '\0';
- return snprintf(page, PAGE_SIZE, "%s\n", str);
+ return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_MODEL_LEN)
+ "s\n", dev->t10_wwn.model);
}
static ssize_t target_stat_lu_rev_show(struct config_item *item, char *page)
{
struct se_device *dev = to_stat_lu_dev(item);
- int i;
- char str[INQUIRY_REVISION_LEN+1];
-
- /* scsiLuRevisionId */
- for (i = 0; i < INQUIRY_REVISION_LEN; i++)
- str[i] = ISPRINT(dev->t10_wwn.revision[i]) ?
- dev->t10_wwn.revision[i] : ' ';
- str[i] = '\0';
- return snprintf(page, PAGE_SIZE, "%s\n", str);
+
+ return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_REVISION_LEN)
+ "s\n", dev->t10_wwn.revision);
}
static ssize_t target_stat_lu_dev_type_show(struct config_item *item, char *page)
--
2.13.7
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps
2018-11-29 1:01 [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps David Disseldorp
@ 2018-11-29 1:33 ` Lee Duncan
2018-11-29 16:37 ` Bart Van Assche
1 sibling, 0 replies; 3+ messages in thread
From: Lee Duncan @ 2018-11-29 1:33 UTC (permalink / raw)
To: target-devel
On 11/28/18 5:01 PM, David Disseldorp wrote:
> From: Bart Van Assche <bvanassche@acm.org>
>
> The existing for loops step over null-terminators for right-padding.
> Padding can be achieved in a much simpler way using printf width
> specifiers.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> drivers/target/target_core_device.c | 35 ++++++++---------------------------
> drivers/target/target_core_stat.c | 32 +++++++-------------------------
> 2 files changed, 15 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index b3d0bd1ab09f..7f2d307e411b 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -720,36 +720,17 @@ void core_dev_free_initiator_node_lun_acl(
> static void scsi_dump_inquiry(struct se_device *dev)
> {
> struct t10_wwn *wwn = &dev->t10_wwn;
> - char buf[INQUIRY_MODEL_LEN + 1];
> - int i, device_type;
> + int device_type = dev->transport->get_device_type(dev);
> +
> /*
> * Print Linux/SCSI style INQUIRY formatting to the kernel ring buffer
> */
> - for (i = 0; i < INQUIRY_VENDOR_LEN; i++)
> - if (wwn->vendor[i] >= 0x20)
> - buf[i] = wwn->vendor[i];
> - else
> - buf[i] = ' ';
> - buf[i] = '\0';
> - pr_debug(" Vendor: %s\n", buf);
> -
> - for (i = 0; i < INQUIRY_MODEL_LEN; i++)
> - if (wwn->model[i] >= 0x20)
> - buf[i] = wwn->model[i];
> - else
> - buf[i] = ' ';
> - buf[i] = '\0';
> - pr_debug(" Model: %s\n", buf);
> -
> - for (i = 0; i < INQUIRY_REVISION_LEN; i++)
> - if (wwn->revision[i] >= 0x20)
> - buf[i] = wwn->revision[i];
> - else
> - buf[i] = ' ';
> - buf[i] = '\0';
> - pr_debug(" Revision: %s\n", buf);
> -
> - device_type = dev->transport->get_device_type(dev);
> + pr_debug(" Vendor: %-" __stringify(INQUIRY_VENDOR_LEN) "s\n",
> + wwn->vendor);
> + pr_debug(" Model: %-" __stringify(INQUIRY_MODEL_LEN) "s\n",
> + wwn->model);
> + pr_debug(" Revision: %-" __stringify(INQUIRY_REVISION_LEN) "s\n",
> + wwn->revision);
> pr_debug(" Type: %s ", scsi_device_type(device_type));
> }
>
> diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
> index e437ba494865..87fd2b11fe3b 100644
> --- a/drivers/target/target_core_stat.c
> +++ b/drivers/target/target_core_stat.c
> @@ -246,43 +246,25 @@ static ssize_t target_stat_lu_lu_name_show(struct config_item *item, char *page)
> static ssize_t target_stat_lu_vend_show(struct config_item *item, char *page)
> {
> struct se_device *dev = to_stat_lu_dev(item);
> - int i;
> - char str[INQUIRY_VENDOR_LEN+1];
>
> - /* scsiLuVendorId */
> - for (i = 0; i < INQUIRY_VENDOR_LEN; i++)
> - str[i] = ISPRINT(dev->t10_wwn.vendor[i]) ?
> - dev->t10_wwn.vendor[i] : ' ';
> - str[i] = '\0';
> - return snprintf(page, PAGE_SIZE, "%s\n", str);
> + return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_VENDOR_LEN)
> + "s\n", dev->t10_wwn.vendor);
> }
>
> static ssize_t target_stat_lu_prod_show(struct config_item *item, char *page)
> {
> struct se_device *dev = to_stat_lu_dev(item);
> - int i;
> - char str[INQUIRY_MODEL_LEN+1];
>
> - /* scsiLuProductId */
> - for (i = 0; i < INQUIRY_MODEL_LEN; i++)
> - str[i] = ISPRINT(dev->t10_wwn.model[i]) ?
> - dev->t10_wwn.model[i] : ' ';
> - str[i] = '\0';
> - return snprintf(page, PAGE_SIZE, "%s\n", str);
> + return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_MODEL_LEN)
> + "s\n", dev->t10_wwn.model);
> }
>
> static ssize_t target_stat_lu_rev_show(struct config_item *item, char *page)
> {
> struct se_device *dev = to_stat_lu_dev(item);
> - int i;
> - char str[INQUIRY_REVISION_LEN+1];
> -
> - /* scsiLuRevisionId */
> - for (i = 0; i < INQUIRY_REVISION_LEN; i++)
> - str[i] = ISPRINT(dev->t10_wwn.revision[i]) ?
> - dev->t10_wwn.revision[i] : ' ';
> - str[i] = '\0';
> - return snprintf(page, PAGE_SIZE, "%s\n", str);
> +
> + return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_REVISION_LEN)
> + "s\n", dev->t10_wwn.revision);
> }
>
> static ssize_t target_stat_lu_dev_type_show(struct config_item *item, char *page)
>
Reviewed-by: Lee Duncan <lduncan@suse.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps
2018-11-29 1:01 [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps David Disseldorp
2018-11-29 1:33 ` Lee Duncan
@ 2018-11-29 16:37 ` Bart Van Assche
1 sibling, 0 replies; 3+ messages in thread
From: Bart Van Assche @ 2018-11-29 16:37 UTC (permalink / raw)
To: target-devel
On Thu, 2018-11-29 at 02:01 +-0100, David Disseldorp wrote:
+AD4 The existing for loops step over null-terminators for right-padding.
+AD4 Padding can be achieved in a much simpler way using printf width
+AD4 specifiers.
How about squashing patches 2, 3, 4 and 7 into a single patch? I think
that would make the entire patch series easier to review.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-29 16:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-29 1:01 [PATCH v4 7/7] target: use printf width specifier for t10_wwn field dumps David Disseldorp
2018-11-29 1:33 ` Lee Duncan
2018-11-29 16:37 ` Bart Van Assche
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.