Re: [PATCH v6 4/6] PCI/sysfs: Add missing trailing newline to devspec_show()

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Sebastian]

On Thu, Jun 03, 2021 at 12:01:10AM +0000, Krzysztof Wilczyński wrote:
> At the moment, when the value of the "devspec" sysfs object is read from
> the user space there will be no newline present, and the utilities such
> as the "cat" command won't display the result of the read correctly in
> a shell, as the trailing newline is currently missing.
> 
> To fix this, append a newline character in the show() function.

There are a few other devspec_show() functions:

  arch/powerpc/platforms/pseries/ibmebus.c
  arch/powerpc/platforms/pseries/vio.c
  arch/sparc/kernel/vio.c
  drivers/usb/core/sysfs.c

and they all include the newline.  So I assume it's not likely that
this minor user-visible change will break something.

I did cc: Sebastian, since his dfc73e7acd99 ("PCI: Move Open Firmware
devspec attribute to PCI common code") moved this code to pci-sysfs.c
in the first place (it came from microblaze and powerpc code that
*also* did not include the newline).

I notice that pci-sysfs.c is the only one that returns 0 if of_node is
NULL.  Probably makes more sense than what the others do.  I'm
guessing the others would print "(null)" which doesn't seem quite
right for a sysfs attribute.

But pci-sysfs.c also goes to a little more work than necessary to look
up of_node:

  struct pci_dev *pdev = to_pci_dev(dev);
  struct device_node *np = pci_device_to_OF_node(pdev);

when I think this would be equivalent:

  struct device_node *np = dev->of_node;

Some of the others do a similar dance with struct platform_device.

Why'd I write all the above?  I dunno; this looks good to me, no
action required for you :)

> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/pci/pci-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index beb8d1f4fafe..5d63df7c1820 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -537,7 +537,7 @@ static ssize_t devspec_show(struct device *dev,
>  
>  	if (np == NULL)
>  		return 0;
> -	return sysfs_emit(buf, "%pOF", np);
> +	return sysfs_emit(buf, "%pOF\n", np);
>  }
>  static DEVICE_ATTR_RO(devspec);
>  #endif
> -- 
> 2.31.1
>