[v2,01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hello,

[...]
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Please disregard this "Reviewed-by" from Logan for this version, as I've
forgotten to remove it before sending v2 after pulling patches using b4.

Apologies.

Krzysztof

On Sat, 2021-05-15 at 05:24 +0000, Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
[]
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
[]
> @@ -6439,7 +6439,7 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf)
>  
> 
>  	spin_lock(&resource_alignment_lock);
>  	if (resource_alignment_param)
> -		count = scnprintf(buf, PAGE_SIZE, "%s", resource_alignment_param);
> +		count = sysfs_emit(buf, "%s", resource_alignment_param);
>  	spin_unlock(&resource_alignment_lock);

Ideally, the additional newline check below this would use sysfs_emit_at

drivers/pci/pci.c-      /*
drivers/pci/pci.c:       * When set by the command line, resource_alignment_param will not
drivers/pci/pci.c-       * have a trailing line feed, which is ugly. So conditionally add
drivers/pci/pci.c-       * it here.
drivers/pci/pci.c-       */
drivers/pci/pci.c-      if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) {
drivers/pci/pci.c-              buf[count - 1] = '\n';
drivers/pci/pci.c-              buf[count++] = 0;
drivers/pci/pci.c-      }
drivers/pci/pci.c-
drivers/pci/pci.c-      return count;

Hi Joe,

[...]
> Ideally, the additional newline check below this would use sysfs_emit_at
> 
> drivers/pci/pci.c-      /*
> drivers/pci/pci.c:       * When set by the command line, resource_alignment_param will not
> drivers/pci/pci.c-       * have a trailing line feed, which is ugly. So conditionally add
> drivers/pci/pci.c-       * it here.
> drivers/pci/pci.c-       */
> drivers/pci/pci.c-      if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) {
> drivers/pci/pci.c-              buf[count - 1] = '\n';
> drivers/pci/pci.c-              buf[count++] = 0;
> drivers/pci/pci.c-      }
> drivers/pci/pci.c-
> drivers/pci/pci.c-      return count;

I found some inconsistencies with adding newline this way, and decided
to change the code slightly, see:

  https://lore.kernel.org/linux-pci/20210515052434.1413236-12-kw@linux.com/

Krzysztof

On 2021-05-14 11:24 p.m., Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
> 
> No functional change intended.
> 
> [1] Documentation/filesystems/sysfs.rst
> 
> Related to:
>   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")
> 
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

I re-reviewed the whole series. It still looks good to me.

Very nice solution in patch 12 to the new line issue.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Thanks,

Logan

Hi Logan,

> > The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> > it less ambiguous which function is preferred when writing to the output
> > buffer in a device attribute's "show" callback [1].
> > 
> > Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> > and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> > latter is aware of the PAGE_SIZE buffer and correctly returns the number
> > of bytes written into the buffer.
> > 
> > No functional change intended.
> > 
> > [1] Documentation/filesystems/sysfs.rst
> > 
> > Related to:
> >   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")
> 
> I re-reviewed the whole series. It still looks good to me.
> 
> Very nice solution in patch 12 to the new line issue.
> 
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> 
> Thanks,

Thank you!

I will send v3 incorporating the style change as per Joe's suggestion
and carry-over your "Reviewed-by", if you don't mind, as it will be
a trivial change.

Krzysztof