diff mbox series

[v4,7/7] target: use printf width specifier for t10_wwn field dumps

Message ID 20181129010157.12687-8-ddiss@suse.de (mailing list archive)
State Superseded
Headers show
Series target: user configurable T10 Vendor ID | expand

Commit Message

David Disseldorp Nov. 29, 2018, 1:01 a.m. UTC 
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(-)

Comments

Lee Duncan Nov. 29, 2018, 1:33 a.m. UTC | #1 
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>
Bart Van Assche Nov. 29, 2018, 4:37 p.m. UTC | #2 
On Thu, 2018-11-29 at 02:01 +0100, David Disseldorp wrote:
> The existing for loops step over null-terminators for right-padding.
> Padding can be achieved in a much simpler way using printf width
> 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.
diff mbox series

Patch

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)