diff mbox series

[v5,2/5] PCI/sysfs: Use return value from dsm_label_utf16s_to_utf8s() directly

Message ID 20210527201650.221944-2-kw@linux.com
State New
Headers show
Series [v5,1/5] PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions | expand

Commit Message

Krzysztof Wilczyński May 27, 2021, 8:16 p.m. UTC 
Modify the function dsm_label_utf16s_to_utf8s() to directly return the
number of bytes written into the buffer so that the strlen() used later
to calculate the length of the buffer can be removed as it would no
longer be needed.

No functional change intended.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  None.
Changes in v4:
  Separated this patch from other trivial sysfs_emit()/sysfs_emit_at()
  patches into a separate patch as per Bjorn Helgaas' request.
  Added Logan Gunthorpe's "Reviewed-by".
Changes in v5:
  None.

 drivers/pci/pci-label.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Joe Perches May 27, 2021, 8:23 p.m. UTC | #1 
On Thu, 2021-05-27 at 20:16 +0000, Krzysztof Wilczyński wrote:
> Modify the function dsm_label_utf16s_to_utf8s() to directly return the
> number of bytes written into the buffer so that the strlen() used later
> to calculate the length of the buffer can be removed as it would no
> longer be needed.
[]
> diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
[]
> @@ -139,14 +139,17 @@ enum acpi_attr_enum {
>  	ACPI_ATTR_INDEX_SHOW,
>  };
>  
> 
> -static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> +static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
>  {
>  	int len;
> +
>  	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
>  			      obj->buffer.length,
>  			      UTF16_LITTLE_ENDIAN,
>  			      buf, PAGE_SIZE);

This should be PAGE_SIZE - 1 no?

>  	buf[len] = '\n';
> +
> +	return len;

return len + 1 ?
Krzysztof Wilczyński May 27, 2021, 9:34 p.m. UTC | #2 
Hi Joe,

[...]
> > -static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> > +static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> >  {
> >  	int len;
> > +
> >  	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
> >  			      obj->buffer.length,
> >  			      UTF16_LITTLE_ENDIAN,
> >  			      buf, PAGE_SIZE);
> 
> This should be PAGE_SIZE - 1 no?
> 
> >  	buf[len] = '\n';
> > +
> > +	return len;
> 
> return len + 1 ?

Good catch!

I left the original code as-is, and this old latent bug would remain
there.  I am glad you had a moment to review this new version.  Thank
you!

I will send v6 later and include this as separate patch per Bjorn's
request.

	Krzysztof
diff mbox series

Patch

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 311dd48e2881..000e169c7197 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -139,14 +139,17 @@  enum acpi_attr_enum {
 	ACPI_ATTR_INDEX_SHOW,
 };
 
-static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
+static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
 {
 	int len;
+
 	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
 			      obj->buffer.length,
 			      UTF16_LITTLE_ENDIAN,
 			      buf, PAGE_SIZE);
 	buf[len] = '\n';
+
+	return len;
 }
 
 static int dsm_get_label(struct device *dev, char *buf,
@@ -154,7 +157,7 @@  static int dsm_get_label(struct device *dev, char *buf,
 {
 	acpi_handle handle = ACPI_HANDLE(dev);
 	union acpi_object *obj, *tmp;
-	int len = -1;
+	int len = 0;
 
 	if (!handle)
 		return -1;
@@ -175,20 +178,19 @@  static int dsm_get_label(struct device *dev, char *buf,
 		 * this entry must return a null string.
 		 */
 		if (attr == ACPI_ATTR_INDEX_SHOW) {
-			sysfs_emit(buf, "%llu\n", tmp->integer.value);
+			len = sysfs_emit(buf, "%llu\n", tmp->integer.value);
 		} else if (attr == ACPI_ATTR_LABEL_SHOW) {
 			if (tmp[1].type == ACPI_TYPE_STRING)
-				sysfs_emit(buf, "%s\n",
-					   tmp[1].string.pointer);
+				len = sysfs_emit(buf, "%s\n",
+						 tmp[1].string.pointer);
 			else if (tmp[1].type == ACPI_TYPE_BUFFER)
-				dsm_label_utf16s_to_utf8s(tmp + 1, buf);
+				len = dsm_label_utf16s_to_utf8s(tmp + 1, buf);
 		}
-		len = strlen(buf) > 0 ? strlen(buf) : -1;
 	}
 
 	ACPI_FREE(obj);
 
-	return len;
+	return len > 0 ? len : -1;
 }
 
 static ssize_t label_show(struct device *dev, struct device_attribute *attr,